error_on_duplicate_columns_in_analyze_v2.patch

application/octet-stream

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

Patch

Format: unified
Series: patch v2
File+
src/backend/commands/vacuum.c 48 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 756be3d..5d99630 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -70,7 +70,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,
@@ -265,8 +265,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,17 +553,24 @@ 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)
 {
+	/*
+	 * First, verify that all specified columns exist.
+	 */
 	ListCell *relation_lc;
 	foreach(relation_lc, relations)
 	{
@@ -606,6 +616,40 @@ check_columns_exist(List *relations)
 		}
 		relation_close(rel, NoLock);
 	}
+
+	/*
+	 * Verify that there are no duplicate entries in any of the column
+	 * lists.
+	 *
+	 * We do this after checking that all columns exist so that nonexistent
+	 * columns ERROR with higher priority than duplicates.
+	 */
+	foreach(relation_lc, relations)
+	{
+		VacuumRelation *relation = lfirst_node(VacuumRelation, relation_lc);
+		List *unique_columns;
+
+		if (relation->va_cols == NIL)	/* nothing to check */
+			continue;
+
+		unique_columns = list_concat_unique(NIL, relation->va_cols);
+		if (list_length(unique_columns) == list_length(relation->va_cols))
+			continue;
+
+		if (relation->relation->schemaname)
+			ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("column lists cannot have duplicate entries"),
+				 errhint("the column list specified for relation \"%s.%s\" contains duplicates",
+					relation->relation->schemaname,
+					relation->relation->relname)));
+		else
+			ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("column lists cannot have duplicate entries"),
+				 errhint("the column list specified for relation \"%s\" contains duplicates",
+					relation->relation->relname)));
+	}
 }
 
 /*
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index d29dd8f..5e8f3d9 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 "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,6 +115,8 @@ 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 "vacparted" does not exist
 VACUUM;
 ANALYZE vactst, vacparted;
 ANALYZE vacparted (b), vactst;
@@ -120,6 +124,11 @@ ANALYZE VERBOSE vactst (i), vacparted (does_not_exist);
 ERROR:  column "does_not_exist" of relation "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 "vactst" contains duplicates
+ANALYZE vacparted, vactst (i, i, i, does_not_exist);
+ERROR:  column "does_not_exist" of relation "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 991bea7..6671bda 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -87,12 +87,15 @@ 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);
 VACUUM;
 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;