error_on_duplicate_columns_in_analyze_v4.patch

application/octet-stream

Filename: error_on_duplicate_columns_in_analyze_v4.patch
Type: application/octet-stream
Part: 1
Message: Re: [Proposal] Allow users to specify multiple tables in VACUUM commands

Patch

Format: unified
Series: patch v4
File+
src/backend/commands/vacuum.c 38 4
src/test/regress/expected/vacuum.out 9 0
src/test/regress/sql/vacuum.sql 3 0
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;