pgsql: Allow multiple tables to be specified in one VACUUM or ANALYZE c

Started by Tom Laneover 8 years ago3 messages
#1Tom Lane
tgl@sss.pgh.pa.us

Allow multiple tables to be specified in one VACUUM or ANALYZE command.

Not much to say about this; does what it says on the tin.

However, formerly, if there was a column list then the ANALYZE action was
implied; now it must be specified, or you get an error. This is because
it would otherwise be a bit unclear what the user meant if some tables
have column lists and some don't.

Nathan Bossart, reviewed by Michael Paquier and Masahiko Sawada, with some
editorialization by me

Discussion: /messages/by-id/E061A8E3-5E3D-494D-94F0-E8A9B312BBFC@amazon.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/11d8d72c27a64ea4e30adce11cf6c4f3dd3e60db

Modified Files
--------------
doc/src/sgml/ref/analyze.sgml | 14 ++-
doc/src/sgml/ref/vacuum.sgml | 36 ++++--
src/backend/commands/vacuum.c | 236 +++++++++++++++++++++++------------
src/backend/nodes/copyfuncs.c | 14 +++
src/backend/nodes/equalfuncs.c | 12 ++
src/backend/nodes/makefuncs.c | 15 +++
src/backend/parser/gram.y | 71 ++++-------
src/backend/postmaster/autovacuum.c | 20 +--
src/include/commands/vacuum.h | 3 +-
src/include/nodes/makefuncs.h | 2 +
src/include/nodes/nodes.h | 1 +
src/include/nodes/parsenodes.h | 22 +++-
src/test/regress/expected/vacuum.out | 23 +++-
src/test/regress/sql/vacuum.sql | 19 ++-
14 files changed, 329 insertions(+), 159 deletions(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#2Michael Paquier
michael.paquier@gmail.com
In reply to: Tom Lane (#1)
1 attachment(s)
Re: [COMMITTERS] pgsql: Allow multiple tables to be specified in one VACUUM or ANALYZE c

On Wed, Oct 4, 2017 at 7:53 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Allow multiple tables to be specified in one VACUUM or ANALYZE command.

Not much to say about this; does what it says on the tin.

However, formerly, if there was a column list then the ANALYZE action was
implied; now it must be specified, or you get an error. This is because
it would otherwise be a bit unclear what the user meant if some tables
have column lists and some don't.

Nathan Bossart, reviewed by Michael Paquier and Masahiko Sawada, with some
editorialization by me

Tom, it seems to me that in the portions you have editorialized, you
have forgotten to update two comments still mentioning get_rel_oids()
in vacuum.c and analyze.c. Those should now refer to
expand_vacuum_rel() instead. Please see the attached.
--
Michael

Attachments:

vacuum-fix-comments.patchapplication/octet-stream; name=vacuum-fix-comments.patchDownload
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index d432f8208d..2b8de85e14 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -208,8 +208,8 @@ analyze_rel(Oid relid, RangeVar *relation, int options,
 
 	/*
 	 * Check that it's a plain table, materialized view, or foreign table; we
-	 * used to do this in get_rel_oids() but seems safer to check after we've
-	 * locked the relation.
+	 * used to do this in expand_vacuum_rel() but seems safer to check after
+	 * we've locked the relation.
 	 */
 	if (onerel->rd_rel->relkind == RELKIND_RELATION ||
 		onerel->rd_rel->relkind == RELKIND_MATVIEW)
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index f439b55ea5..3db96cac11 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1445,7 +1445,7 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params)
 
 	/*
 	 * Check that it's a vacuumable relation; we used to do this in
-	 * get_rel_oids() but seems safer to check after we've locked the
+	 * expand_vacuum_rel() but seems safer to check after we've locked the
 	 * relation.
 	 */
 	if (onerel->rd_rel->relkind != RELKIND_RELATION &&
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#2)
Re: [COMMITTERS] pgsql: Allow multiple tables to be specified in one VACUUM or ANALYZE c

Michael Paquier <michael.paquier@gmail.com> writes:

Tom, it seems to me that in the portions you have editorialized, you
have forgotten to update two comments still mentioning get_rel_oids()
in vacuum.c and analyze.c. Those should now refer to
expand_vacuum_rel() instead. Please see the attached.

Oh, good point. I don't think that just s/get_rel_oids/expand_vacuum_rel/
will do though, as it leaves out the interaction with get_all_vacuum_rels.
I decided that the point was of merely historical interest anyway, and
just removed the reference to the other routine.

The partitioned-table patches seem to have been a bit sloppy with
maintaining these comments too, so I ended up doing more than that...

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers