diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index d46e686e6c..c1ff4d7e7f 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -71,7 +71,7 @@ static BufferAccessStrategy vac_strategy; /* non-export function prototypes */ static void get_rel_oids(List **vacrels); -static void check_columns_exist(List *relations); +static void check_column_lists(List *relations); static void vac_truncate_clog(TransactionId frozenXID, MultiXactId minMulti, TransactionId lastSaneFrozenXid, @@ -266,8 +266,11 @@ vacuum(int options, List *relations, VacuumParams *params, * Check that all specified columns exist so that we can fast-fail * commands with multiple tables. If the column disappears before we * actually process it, we will emit a WARNING and skip it later on. + * + * Also verify that there are no duplicate columns specified in any of + * the column lists. */ - check_columns_exist(relations); + check_column_lists(relations); /* * Decide whether we need to start/commit our own transactions. @@ -550,16 +553,20 @@ get_rel_oids(List **vacrels) } /* - * Check that all specified columns for the relations exist. + * Check that all specified columns for the relations exist and that there are + * no duplicate columns specified in each list. * * This function emits an ERROR if the relation or the columns specified for * a relation cannot be found. This is used to pre-validate any column lists * specified in VACUUM/ANALYZE commands. If any columns disappear between * calling this function and when we actually get to processing them, we will * emit a WARNING and skip it at that time. + * + * This function also emits an ERROR if any column list contains duplicate + * entries, as ANALYZE will fail in that case. */ static void -check_columns_exist(List *relations) +check_column_lists(List *relations) { ListCell *relation_lc; foreach(relation_lc, relations) @@ -568,6 +575,7 @@ check_columns_exist(List *relations) Relation rel; ListCell *column_lc; char *schemaname; + List *unique_columns; if (relation->va_cols == NIL) /* nothing to check */ continue; @@ -596,6 +604,9 @@ check_columns_exist(List *relations) schemaname = get_namespace_name(RelationGetNamespace(rel)); + /* + * First, verify that all specified columns exist. + */ foreach(column_lc, relation->va_cols) { char *col = strVal(lfirst(column_lc)); @@ -615,6 +626,29 @@ check_columns_exist(List *relations) errmsg("column \%s\" of relation \"%s\" does not exist", col, RelationGetRelationName(rel)))); } + + /* + * Verify that there are no duplicate entries in the column list. + * + * We do this after checking that all columns exist so that nonexistent + * columns ERROR with higher priority than duplicates. + */ + unique_columns = list_concat_unique(NIL, relation->va_cols); + if (list_length(unique_columns) != list_length(relation->va_cols)) + { + if (schemaname) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_COLUMN), + errmsg("column lists cannot have duplicate entries"), + errhint("the column list specified for relation \"%s.%s\" contains duplicates", + schemaname, RelationGetRelationName(rel)))); + else + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_COLUMN), + errmsg("column lists cannot have duplicate entries"), + errhint("the column list specified for relation \"%s\" contains duplicates", + RelationGetRelationName(rel)))); + } relation_close(rel, NoLock); } } diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index 62c1a03685..4272d1136c 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -97,6 +97,8 @@ VACUUM (FREEZE) vaccluster, does_not_exist; ERROR: relation "does_not_exist" does not exist VACUUM (FREEZE, ANALYZE) vactst (i); VACUUM (FREEZE, ANALYZE) vactst (i, i, i); +ERROR: column lists cannot have duplicate entries +HINT: the column list specified for relation "public.vactst" contains duplicates VACUUM (FREEZE, ANALYZE) vactst, vacparted (a); VACUUM (FREEZE, ANALYZE) vacparted, vactst (i), vacparted; VACUUM (FREEZE, ANALYZE) vactst (i), vacparted (a, b), vaccluster (i); @@ -113,12 +115,19 @@ HINT: A column list was specified for relation "vaccluster". VACUUM (VERBOSE) vactst (i), vacparted (a, b), vaccluster (i); ERROR: ANALYZE option must be specified when a column list is provided HINT: A column list was specified for relation "vactst". +VACUUM (ANALYZE) vactst, vacparted (does_not_exist, does_not_exist); +ERROR: column "does_not_exist" of relation "public.vacparted" does not exist ANALYZE vactst, vacparted; ANALYZE vacparted (b), vactst; ANALYZE VERBOSE vactst (i), vacparted (does_not_exist); ERROR: column "does_not_exist" of relation "public.vacparted" does not exist ANALYZE vactst, vactst, vactst; ANALYZE vacparted (a), vacparted (b), vacparted; +ANALYZE vactst (i, i), vacparted (a), vactst (i), vacparted (a, b, a); +ERROR: column lists cannot have duplicate entries +HINT: the column list specified for relation "public.vactst" contains duplicates +ANALYZE vacparted, vactst (i, i, i, does_not_exist); +ERROR: column "does_not_exist" of relation "public.vactst" does not exist DROP TABLE vaccluster; DROP TABLE vactst; DROP TABLE vacparted; diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql index 10e0de0113..6ccba3b8c5 100644 --- a/src/test/regress/sql/vacuum.sql +++ b/src/test/regress/sql/vacuum.sql @@ -87,11 +87,14 @@ VACUUM (FREEZE, ANALYZE) vacparted (a), vacparted (b), vacparted; VACUUM vactst (i); VACUUM (VERBOSE) vactst, vacparted, vaccluster (i); VACUUM (VERBOSE) vactst (i), vacparted (a, b), vaccluster (i); +VACUUM (ANALYZE) vactst, vacparted (does_not_exist, does_not_exist); ANALYZE vactst, vacparted; ANALYZE vacparted (b), vactst; ANALYZE VERBOSE vactst (i), vacparted (does_not_exist); ANALYZE vactst, vactst, vactst; ANALYZE vacparted (a), vacparted (b), vacparted; +ANALYZE vactst (i, i), vacparted (a), vactst (i), vacparted (a, b, a); +ANALYZE vacparted, vactst (i, i, i, does_not_exist); DROP TABLE vaccluster; DROP TABLE vactst;