From 55cfa77237608adac12d03c4307356b3c91907cc Mon Sep 17 00:00:00 2001 From: Zubeyr Eryilmaz Date: Mon, 11 Nov 2024 20:23:16 +0000 Subject: [PATCH] Issue #422: Make --error-on-invalid-index the default behavior Skipping repacking the invalid indexes could lead to corrupting the invalid indexes. In some rare cases, this might cause inserts to tables to fail as changes to a table continuously be applied to invalid indexes on it and updates to corrupted invalid indexes potentially might fail by the checks done during insertion. Therefore, by default do not repack a table with invalid indexes. Repack only user explicitly specifies --no-error-on-invalid-index. --- bin/pg_repack.c | 51 +++++++++------- doc/pg_repack.rst | 61 ++++++++++--------- doc/pg_repack_jp.rst | 59 +++++++++--------- regress/Makefile | 2 +- regress/expected/no-error-on-invalid-idx.out | 22 +++++++ .../expected/no-error-on-invalid-idx_1.out | 21 +++++++ regress/expected/repack-run.out | 4 +- regress/expected/repack-run_1.out | 4 +- regress/sql/no-error-on-invalid-idx.sql | 7 +++ 9 files changed, 144 insertions(+), 87 deletions(-) create mode 100644 regress/expected/no-error-on-invalid-idx.out create mode 100644 regress/expected/no-error-on-invalid-idx_1.out create mode 100644 regress/sql/no-error-on-invalid-idx.sql diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 6b9cde69..5dc377aa 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -258,7 +258,9 @@ 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 error_on_invalid_index = false; /* don't repack when invalid index is found */ +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, + * deprecated, this the default behavior now */ static int apply_count = APPLY_COUNT_DEFAULT; static int switch_threshold = SWITCH_THRESHOLD_DEFAULT; @@ -290,6 +292,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 +1314,7 @@ 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 --no-error-on-invalid-index is set */ indexres = execute( "SELECT pg_get_indexdef(indexrelid)" @@ -1322,7 +1325,8 @@ repack_one_table(repack_table *table, const char *orderby) { const char *indexdef; indexdef = getstr(indexres, j, 0); - if (error_on_invalid_index) { + + if (!no_error_on_invalid_index) { elog(WARNING, "Invalid index: %s", indexdef); goto cleanup; } else { @@ -2402,24 +2406,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, deprecated, as this is the default behavior now\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/doc/pg_repack.rst b/doc/pg_repack.rst index 63b9d395..ebcaa541 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -109,40 +109,41 @@ Usage The following options can be specified in ``OPTIONS``. Options: - -a, --all repack all databases - -t, --table=TABLE repack specific table only - -I, --parent-table=TABLE repack specific parent table and its inheritors - -c, --schema=SCHEMA repack tables in specific schema only - -s, --tablespace=TBLSPC move repacked tables to a new tablespace - -S, --moveidx move repacked indexes to *TBLSPC* too - -o, --order-by=COLUMNS order by columns instead of cluster keys - -n, --no-order do vacuum full instead of cluster - -N, --dry-run print what would have been repacked and exit - -j, --jobs=NUM Use this many parallel jobs for each table - -i, --index=INDEX move only the specified index - -x, --only-indexes move only indexes of the specified table - -T, --wait-timeout=SECS timeout to cancel other backends on conflict - -D, --no-kill-backend don't kill other backends when timed out - -Z, --no-analyze don't analyze at end - -k, --no-superuser-check skip superuser checks in client - -C, --exclude-extension don't repack tables which belong to specific extension - --error-on-invalid-index don't repack when invalid index is found - --apply-count number of tuples to apply in one trasaction during replay - --switch-threshold switch tables when that many tuples are left to catchup + -a, --all repack all databases + -t, --table=TABLE repack specific table only + -I, --parent-table=TABLE repack specific parent table and its inheritors + -c, --schema=SCHEMA repack tables in specific schema only + -s, --tablespace=TBLSPC move repacked tables to a new tablespace + -S, --moveidx move repacked indexes to *TBLSPC* too + -o, --order-by=COLUMNS order by columns instead of cluster keys + -n, --no-order do vacuum full instead of cluster + -N, --dry-run print what would have been repacked and exit + -j, --jobs=NUM Use this many parallel jobs for each table + -i, --index=INDEX move only the specified index + -x, --only-indexes move only indexes of the specified table + -T, --wait-timeout=SECS timeout to cancel other backends on conflict + -D, --no-kill-backend don't kill other backends when timed out + -Z, --no-analyze don't analyze at end + -k, --no-superuser-check skip superuser checks in client + -C, --exclude-extension don't repack tables which belong to specific extension + --no-error-on-invalid-index repack even though invalid index is found + --error-on-invalid-index don't repack when invalid index is found, deprecated, as this is the default behavior now + --apply-count number of tuples to apply in one trasaction during replay + --switch-threshold switch tables when that many tuples are left to catchup Connection options: - -d, --dbname=DBNAME database to connect - -h, --host=HOSTNAME database server host or socket directory - -p, --port=PORT database server port - -U, --username=USERNAME user name to connect as - -w, --no-password never prompt for password - -W, --password force password prompt + -d, --dbname=DBNAME database to connect + -h, --host=HOSTNAME database server host or socket directory + -p, --port=PORT database server port + -U, --username=USERNAME user name to connect as + -w, --no-password never prompt for password + -W, --password force password prompt Generic options: - -e, --echo echo queries - -E, --elevel=LEVEL set output message level - --help show this help, then exit - --version output version information, then exit + -e, --echo echo queries + -E, --elevel=LEVEL set output message level + --help show this help, then exit + --version output version information, then exit Reorg Options diff --git a/doc/pg_repack_jp.rst b/doc/pg_repack_jp.rst index e8695290..bf2d0fb7 100644 --- a/doc/pg_repack_jp.rst +++ b/doc/pg_repack_jp.rst @@ -197,39 +197,40 @@ pg_repackもしくはpg_reorgの古いバージョンからのアップグレー The following options can be specified in ``OPTIONS``. Options: - -a, --all repack all databases - -t, --table=TABLE repack specific table only - -I, --parent-table=TABLE repack specific parent table and its inheritors - -c, --schema=SCHEMA repack tables in specific schema only - -s, --tablespace=TBLSPC move repacked tables to a new tablespace - -S, --moveidx move repacked indexes to *TBLSPC* too - -o, --order-by=COLUMNS order by columns instead of cluster keys - -n, --no-order do vacuum full instead of cluster - -N, --dry-run print what would have been repacked and exit - -j, --jobs=NUM Use this many parallel jobs for each table - -i, --index=INDEX move only the specified index - -x, --only-indexes move only indexes of the specified table - -T, --wait-timeout=SECS timeout to cancel other backends on conflict - -D, --no-kill-backend don't kill other backends when timed out - -Z, --no-analyze don't analyze at end - -k, --no-superuser-check skip superuser checks in client - -C, --exclude-extension don't repack tables which belong to specific extension - --error-on-invalid-index don't repack when invalid index is found - --switch-threshold switch tables when that many tuples are left to catchup + -a, --all repack all databases + -t, --table=TABLE repack specific table only + -I, --parent-table=TABLE repack specific parent table and its inheritors + -c, --schema=SCHEMA repack tables in specific schema only + -s, --tablespace=TBLSPC move repacked tables to a new tablespace + -S, --moveidx move repacked indexes to *TBLSPC* too + -o, --order-by=COLUMNS order by columns instead of cluster keys + -n, --no-order do vacuum full instead of cluster + -N, --dry-run print what would have been repacked and exit + -j, --jobs=NUM Use this many parallel jobs for each table + -i, --index=INDEX move only the specified index + -x, --only-indexes move only indexes of the specified table + -T, --wait-timeout=SECS timeout to cancel other backends on conflict + -D, --no-kill-backend don't kill other backends when timed out + -Z, --no-analyze don't analyze at end + -k, --no-superuser-check skip superuser checks in client + -C, --exclude-extension don't repack tables which belong to specific extension + --no-error-on-invalid-index repack even though invalid index is found + --error-on-invalid-index don't repack when invalid index is found, deprecated, as this is the default behavior now + --switch-threshold switch tables when that many tuples are left to catchup Connection options: - -d, --dbname=DBNAME database to connect - -h, --host=HOSTNAME database server host or socket directory - -p, --port=PORT database server port - -U, --username=USERNAME user name to connect as - -w, --no-password never prompt for password - -W, --password force password prompt + -d, --dbname=DBNAME database to connect + -h, --host=HOSTNAME database server host or socket directory + -p, --port=PORT database server port + -U, --username=USERNAME user name to connect as + -w, --no-password never prompt for password + -W, --password force password prompt Generic options: - -e, --echo echo queries - -E, --elevel=LEVEL set output message level - --help show this help, then exit - --version output version information, then exit + -e, --echo echo queries + -E, --elevel=LEVEL set output message level + --help show this help, then exit + --version output version information, then exit 利用方法 --------- diff --git a/regress/Makefile b/regress/Makefile index b29c784e..bf6edcb4 100644 --- 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 new file mode 100644 index 00000000..de0dfc43 --- /dev/null +++ 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 new file mode 100644 index 00000000..0bdf8f0d --- /dev/null +++ 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 index 3c734ba2..ebdffa16 100644 --- 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 index b5dbd6e1..e655eabd 100644 --- 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 new file mode 100644 index 00000000..d7f00d1f --- /dev/null +++ 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