Issue reorg#422: Make --error-on-invalid-index the default behavior

zubeyr94 · Nov 11, 2024 · d1631fd · d1631fd
1 parent 85b64c6
commit d1631fd
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 27 deletions.
diff --git a/bin/pg_repack.c b/bin/pg_repack.c
@@ -258,6 +258,7 @@ static unsigned int		temp_obj_num = 0; /* temporary objects counter */
 static bool				no_kill_backend = false; /* abandon when timed-out */
 static bool				no_superuser_check = false;
 static SimpleStringList	exclude_extension_list = {NULL, NULL}; /* don't repack tables of these extensions */
+static bool 			no_error_on_invalid_index = false; /* repack even though invalid index is found */
 static bool 			error_on_invalid_index = false; /* don't repack when invalid index is found */
 static int				apply_count = APPLY_COUNT_DEFAULT;
 static int				switch_threshold = SWITCH_THRESHOLD_DEFAULT;
@@ -290,6 +291,7 @@ static pgut_option options[] =
 	{ 'b', 'D', "no-kill-backend", &no_kill_backend },
 	{ 'b', 'k', "no-superuser-check", &no_superuser_check },
 	{ 'l', 'C', "exclude-extension", &exclude_extension_list },
+	{ 'b', 4, "no-error-on-invalid-index", &no_error_on_invalid_index },
 	{ 'b', 3, "error-on-invalid-index", &error_on_invalid_index },
 	{ 'i', 2, "apply-count", &apply_count },
 	{ 'i', 1, "switch-threshold", &switch_threshold },
@@ -1311,7 +1313,8 @@ repack_one_table(repack_table *table, const char *orderby)
 
 	/* First, just display a warning message for any invalid indexes
 	 * which may be on the table (mostly to match the behavior of 1.1.8),
-	 * if --error-on-invalid-index is not set
+	 * if --error-on-invalid-index is not set and 
+	 * --no-error-on-invalid-index is set
 	 */
 	indexres = execute(
 		"SELECT pg_get_indexdef(indexrelid)"
@@ -1322,7 +1325,11 @@ repack_one_table(repack_table *table, const char *orderby)
 	{
 		const char *indexdef;
 		indexdef = getstr(indexres, j, 0);
-		if (error_on_invalid_index) {
+
+		/* Enforce --error-on-invalid-index in all cases unless
+		 * --no_error_on_invalid_index passed by itself
+		 */
+		if (error_on_invalid_index || !no_error_on_invalid_index) {
 			elog(WARNING, "Invalid index: %s", indexdef);
 			goto cleanup;
 		} else {
@@ -2402,24 +2409,25 @@ pgut_help(bool details)
 		return;
 
 	printf("Options:\n");
-	printf("  -a, --all                     repack all databases\n");
-	printf("  -t, --table=TABLE             repack specific table only\n");
-	printf("  -I, --parent-table=TABLE      repack specific parent table and its inheritors\n");
-	printf("  -c, --schema=SCHEMA           repack tables in specific schema only\n");
-	printf("  -s, --tablespace=TBLSPC       move repacked tables to a new tablespace\n");
-	printf("  -S, --moveidx                 move repacked indexes to TBLSPC too\n");
-	printf("  -o, --order-by=COLUMNS        order by columns instead of cluster keys\n");
-	printf("  -n, --no-order                do vacuum full instead of cluster\n");
-	printf("  -N, --dry-run                 print what would have been repacked\n");
-	printf("  -j, --jobs=NUM                Use this many parallel jobs for each table\n");
-	printf("  -i, --index=INDEX             move only the specified index\n");
-	printf("  -x, --only-indexes            move only indexes of the specified table\n");
-	printf("  -T, --wait-timeout=SECS       timeout to cancel other backends on conflict\n");
-	printf("  -D, --no-kill-backend         don't kill other backends when timed out\n");
-	printf("  -Z, --no-analyze              don't analyze at end\n");
-	printf("  -k, --no-superuser-check      skip superuser checks in client\n");
-	printf("  -C, --exclude-extension       don't repack tables which belong to specific extension\n");
-	printf("      --error-on-invalid-index  don't repack when invalid index is found\n");
-	printf("      --apply-count             number of tuples to apply in one transaction during replay\n");
-	printf("      --switch-threshold        switch tables when that many tuples are left to catchup\n");
+	printf("  -a, --all                          repack all databases\n");
+	printf("  -t, --table=TABLE                  repack specific table only\n");
+	printf("  -I, --parent-table=TABLE           repack specific parent table and its inheritors\n");
+	printf("  -c, --schema=SCHEMA                repack tables in specific schema only\n");
+	printf("  -s, --tablespace=TBLSPC            move repacked tables to a new tablespace\n");
+	printf("  -S, --moveidx                      move repacked indexes to TBLSPC too\n");
+	printf("  -o, --order-by=COLUMNS             order by columns instead of cluster keys\n");
+	printf("  -n, --no-order                     do vacuum full instead of cluster\n");
+	printf("  -N, --dry-run                      print what would have been repacked\n");
+	printf("  -j, --jobs=NUM                     Use this many parallel jobs for each table\n");
+	printf("  -i, --index=INDEX                  move only the specified index\n");
+	printf("  -x, --only-indexes                 move only indexes of the specified table\n");
+	printf("  -T, --wait-timeout=SECS            timeout to cancel other backends on conflict\n");
+	printf("  -D, --no-kill-backend              don't kill other backends when timed out\n");
+	printf("  -Z, --no-analyze                   don't analyze at end\n");
+	printf("  -k, --no-superuser-check           skip superuser checks in client\n");
+	printf("  -C, --exclude-extension            don't repack tables which belong to specific extension\n");
+	printf("      --no-error-on-invalid-index    repack even though invalid index is found\n");
+	printf("      --error-on-invalid-index       don't repack when invalid index is found\n");
+	printf("      --apply-count                  number of tuples to apply in one transaction during replay\n");
+	printf("      --switch-threshold             switch tables when that many tuples are left to catchup\n");
 }
diff --git a/regress/Makefile b/regress/Makefile
@@ -17,7 +17,7 @@ INTVERSION := $(shell echo $$(($$(echo $(VERSION).0 | sed 's/\([[:digit:]]\{1,\}
 # Test suite
 #
 
-REGRESS := init-extension repack-setup repack-run error-on-invalid-idx after-schema repack-check nosuper tablespace get_order_by trigger
+REGRESS := init-extension repack-setup repack-run error-on-invalid-idx no-error-on-invalid-idx after-schema repack-check nosuper tablespace get_order_by trigger
 
 USE_PGXS = 1	# use pgxs if not in contrib directory
 PGXS := $(shell $(PG_CONFIG) --pgxs)

diff --git a/regress/expected/no-error-on-invalid-idx.out b/regress/expected/no-error-on-invalid-idx.out
@@ -0,0 +1,22 @@
+--
+-- do repack
+--
+\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-error-on-invalid-index
+INFO: repacking table "public.tbl_cluster"
+\! pg_repack --dbname=contrib_regression --table=tbl_badindex --no-error-on-invalid-index
+INFO: repacking table "public.tbl_badindex"
+WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
+\! pg_repack --dbname=contrib_regression --no-error-on-invalid-index
+INFO: repacking table "public.tbl_badindex"
+WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
+INFO: repacking table "public.tbl_cluster"
+INFO: repacking table "public.tbl_gistkey"
+INFO: repacking table "public.tbl_idxopts"
+INFO: repacking table "public.tbl_incl_pkey"
+INFO: repacking table "public.tbl_only_pkey"
+INFO: repacking table "public.tbl_order"
+INFO: repacking table "public.tbl_storage_plain"
+INFO: repacking table "public.tbl_with_dropped_column"
+INFO: repacking table "public.tbl_with_dropped_toast"
+INFO: repacking table "public.tbl_with_mod_column_storage"
+INFO: repacking table "public.tbl_with_toast"
diff --git a/regress/expected/no-error-on-invalid-idx_1.out b/regress/expected/no-error-on-invalid-idx_1.out
@@ -0,0 +1,21 @@
+--
+-- do repack
+--
+\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-error-on-invalid-index
+INFO: repacking table "public.tbl_cluster"
+\! pg_repack --dbname=contrib_regression --table=tbl_badindex --no-error-on-invalid-index
+INFO: repacking table "public.tbl_badindex"
+WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
+\! pg_repack --dbname=contrib_regression --no-error-on-invalid-index
+INFO: repacking table "public.tbl_badindex"
+WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
+INFO: repacking table "public.tbl_cluster"
+INFO: repacking table "public.tbl_gistkey"
+INFO: repacking table "public.tbl_idxopts"
+INFO: repacking table "public.tbl_only_pkey"
+INFO: repacking table "public.tbl_order"
+INFO: repacking table "public.tbl_storage_plain"
+INFO: repacking table "public.tbl_with_dropped_column"
+INFO: repacking table "public.tbl_with_dropped_toast"
+INFO: repacking table "public.tbl_with_mod_column_storage"
+INFO: repacking table "public.tbl_with_toast"
diff --git a/regress/expected/repack-run.out b/regress/expected/repack-run.out
@@ -5,10 +5,10 @@
 INFO: repacking table "public.tbl_cluster"
 \! pg_repack --dbname=contrib_regression --table=tbl_badindex
 INFO: repacking table "public.tbl_badindex"
-WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
+WARNING: Invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
 \! pg_repack --dbname=contrib_regression
 INFO: repacking table "public.tbl_badindex"
-WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
+WARNING: Invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
 INFO: repacking table "public.tbl_cluster"
 INFO: repacking table "public.tbl_gistkey"
 INFO: repacking table "public.tbl_idxopts"

diff --git a/regress/expected/repack-run_1.out b/regress/expected/repack-run_1.out
@@ -5,10 +5,10 @@
 INFO: repacking table "public.tbl_cluster"
 \! pg_repack --dbname=contrib_regression --table=tbl_badindex
 INFO: repacking table "public.tbl_badindex"
-WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
+WARNING: Invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
 \! pg_repack --dbname=contrib_regression
 INFO: repacking table "public.tbl_badindex"
-WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
+WARNING: Invalid index: CREATE UNIQUE INDEX idx_badindex_n ON public.tbl_badindex USING btree (n)
 INFO: repacking table "public.tbl_cluster"
 INFO: repacking table "public.tbl_gistkey"
 INFO: repacking table "public.tbl_idxopts"

diff --git a/regress/sql/no-error-on-invalid-idx.sql b/regress/sql/no-error-on-invalid-idx.sql
@@ -0,0 +1,7 @@
+--
+-- do repack
+--
+
+\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-error-on-invalid-index
+\! pg_repack --dbname=contrib_regression --table=tbl_badindex --no-error-on-invalid-index
+\! pg_repack --dbname=contrib_regression --no-error-on-invalid-index