From 1b3b361dea0ac107a60cb36098971f191552e4cc Mon Sep 17 00:00:00 2001 From: melkij Date: Tue, 17 Mar 2026 13:51:37 +0300 Subject: [PATCH 1/4] pg_repack is aborted with error if a publication FOR ALL TABLES exists and apply_log requires deleting or updating a row in a temporary table. ERROR: cannot update table "table_16388" because it does not have a replica identity and publishes updates It's not that difficult to format `ALTER TABLE repack.table_' || R.oid || ' REPLICA IDENTITY USING INDEX index_' || PK.indexrelid` in repack.tables, but for this to work, I also need to set NOT NULL on columns, which is less trivial. Furthermore, repack might break subscribers of this publication. Given the lack of previous bug reports (this setup never worked since introduction of publications in pg10), I suggest adding a FOR ALL TABLES check for publications and simply refusing to work in such a database by default. --- bin/pg_repack.c | 39 ++++++++++++++++++++++++++++++ regress/Makefile | 2 +- regress/expected/publication.out | 9 +++++++ regress/expected/publication_1.out | 15 ++++++++++++ regress/sql/publication.sql | 10 ++++++++ 5 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 regress/expected/publication.out create mode 100644 regress/expected/publication_1.out create mode 100644 regress/sql/publication.sql diff --git a/bin/pg_repack.c b/bin/pg_repack.c index ac8453bb..49e12d6a 100644 --- 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_publication = false; /* repack even though publication FOR ALL TABLES 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 */ @@ -292,6 +293,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', 5, "no-error-on-publicationn", &no_error_on_publication }, { '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 }, @@ -559,6 +561,42 @@ preliminary_checks(char *errbuf, size_t errsize){ } CLEARPGRES(res); + /* + * repack will be aborted with error like + * + * ERROR: cannot update table "table_16388" because it does not + * have a replica identity and publishes updates + * + * if a publication FOR ALL TABLES exists and apply_log requires + * deleting or updating a row in a temp table. To make this works, + * we need to set NOT NULL on columns of primary key and command + * + * ALTER TABLE repack.table_' || R.oid || ' REPLICA IDENTITY USING INDEX index_' || PK.indexrelid + * + * In addition, repack might break subscribers of this publication. + * For now, we simply refuse to continue. + */ + if (PQserverVersion(connection) >= 100000 && !no_error_on_publication) { + res = execute_elevel("SELECT pubname FROM pg_publication WHERE puballtables LIMIT 1", + 0, NULL, DEBUG2); + if (PQresultStatus(res) == PGRES_TUPLES_OK) + { + if (PQntuples(res) > 0) + { + const char *pubname; + pubname = getstr(res, 0, 0); + snprintf(errbuf, errsize, "publication \"%s\" FOR ALL TABLES found: won't be able to apply concurrent UPDATE and DELETE", pubname); + goto cleanup; + } + /* no publications, continue to work */ + } else { + if (errbuf) + snprintf(errbuf, errsize, "%s", PQerrorMessage(connection)); + goto cleanup; + } + CLEARPGRES(res); + } + /* Disable statement timeout. */ command("SET statement_timeout = 0", 0, NULL); @@ -2450,6 +2488,7 @@ pgut_help(bool details) 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-publication repack even though a publication FOR ALL TABLES is found\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"); diff --git a/regress/Makefile b/regress/Makefile index bf6edcb4..b9462f79 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 no-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 publication USE_PGXS = 1 # use pgxs if not in contrib directory PGXS := $(shell $(PG_CONFIG) --pgxs) diff --git a/regress/expected/publication.out b/regress/expected/publication.out new file mode 100644 index 00000000..3fc6a498 --- /dev/null +++ b/regress/expected/publication.out @@ -0,0 +1,9 @@ +-- +-- repack will conflict with publication FOR ALL TABLES +-- +create publication test for all tables ; +\! pg_repack --dbname=contrib_regression --table=tbl_cluster +ERROR: pg_repack failed with error: publication "test" FOR ALL TABLES found: won't be able to apply concurrent UPDATE and DELETE +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-error-on-publication +INFO: repacking table "public.tbl_cluster" +drop publication test; diff --git a/regress/expected/publication_1.out b/regress/expected/publication_1.out new file mode 100644 index 00000000..22524a77 --- /dev/null +++ b/regress/expected/publication_1.out @@ -0,0 +1,15 @@ +-- +-- repack will conflict with publication FOR ALL TABLES +-- +create publication test for all tables ; +ERROR: syntax error at or near "publication" +LINE 1: create publication test for all tables ; + ^ +\! pg_repack --dbname=contrib_regression --table=tbl_cluster +INFO: repacking table "public.tbl_cluster" +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-error-on-publication +INFO: repacking table "public.tbl_cluster" +drop publication test; +ERROR: syntax error at or near "publication" +LINE 1: drop publication test; + ^ diff --git a/regress/sql/publication.sql b/regress/sql/publication.sql new file mode 100644 index 00000000..08f8629a --- /dev/null +++ b/regress/sql/publication.sql @@ -0,0 +1,10 @@ +-- +-- repack will conflict with publication FOR ALL TABLES +-- + +create publication test for all tables ; + +\! pg_repack --dbname=contrib_regression --table=tbl_cluster +\! pg_repack --dbname=contrib_regression --table=tbl_cluster --no-error-on-publication + +drop publication test; From ae6766e7ad62dfc6d430b38f4d1c9fd6767bf19e Mon Sep 17 00:00:00 2001 From: melkij Date: Tue, 17 Mar 2026 14:07:56 +0300 Subject: [PATCH 2/4] new test require wal_level=logical --- .github/workflows/regression.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml index b6ff888f..3d1be41e 100644 --- a/.github/workflows/regression.yml +++ b/.github/workflows/regression.yml @@ -25,7 +25,7 @@ jobs: steps: - name: Start PostgreSQL ${{ matrix.pg }} - run: pg-start ${{ matrix.pg }} + run: CREATE_OPTIONS="--pgoption wal_level=logical" pg-start ${{ matrix.pg }} - name: Install build-dependencies run: apt-get install -y liblz4-dev libreadline-dev zlib1g-dev libzstd-dev From bf1c6fea639a440e877aca2266250ed32026845f Mon Sep 17 00:00:00 2001 From: Melkij Date: Fri, 20 Mar 2026 11:13:27 +0300 Subject: [PATCH 3/4] correct code style formatting Mostly replacing spaces with tabs. Recently, I've been working on code that used spaces, and the text editor remains in that mode. Co-authored-by: Artur Zakirov --- bin/pg_repack.c | 72 ++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 49e12d6a..96ed1da8 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -561,42 +561,42 @@ preliminary_checks(char *errbuf, size_t errsize){ } CLEARPGRES(res); - /* - * repack will be aborted with error like - * - * ERROR: cannot update table "table_16388" because it does not - * have a replica identity and publishes updates - * - * if a publication FOR ALL TABLES exists and apply_log requires - * deleting or updating a row in a temp table. To make this works, - * we need to set NOT NULL on columns of primary key and command - * - * ALTER TABLE repack.table_' || R.oid || ' REPLICA IDENTITY USING INDEX index_' || PK.indexrelid - * - * In addition, repack might break subscribers of this publication. - * For now, we simply refuse to continue. - */ - if (PQserverVersion(connection) >= 100000 && !no_error_on_publication) { - res = execute_elevel("SELECT pubname FROM pg_publication WHERE puballtables LIMIT 1", - 0, NULL, DEBUG2); - if (PQresultStatus(res) == PGRES_TUPLES_OK) - { - if (PQntuples(res) > 0) - { - const char *pubname; - pubname = getstr(res, 0, 0); - snprintf(errbuf, errsize, "publication \"%s\" FOR ALL TABLES found: won't be able to apply concurrent UPDATE and DELETE", pubname); - goto cleanup; - } - /* no publications, continue to work */ - } else { - if (errbuf) - snprintf(errbuf, errsize, "%s", PQerrorMessage(connection)); - goto cleanup; - } - CLEARPGRES(res); - } - + /* + * repack will be aborted with error like + * + * ERROR: cannot update table "table_16388" because it does not + * have a replica identity and publishes updates + * + * if a publication FOR ALL TABLES exists and apply_log requires + * deleting or updating a row in a temp table. To make this works, + * we need to set NOT NULL on columns of primary key and command + * + * ALTER TABLE repack.table_' || R.oid || ' REPLICA IDENTITY USING INDEX index_' || PK.indexrelid + * + * In addition, repack might break subscribers of this publication. + * For now, we simply refuse to continue. + */ + if (PQserverVersion(connection) >= 100000 && !no_error_on_publication) + { + res = execute_elevel("SELECT pubname FROM pg_publication WHERE puballtables LIMIT 1", + 0, NULL, DEBUG2); + if (PQresultStatus(res) == PGRES_TUPLES_OK) + { + if (PQntuples(res) > 0) + { + const char *pubname; + pubname = getstr(res, 0, 0); + snprintf(errbuf, errsize, "publication \"%s\" FOR ALL TABLES found: won't be able to apply concurrent UPDATE and DELETE", pubname); + goto cleanup; + } + /* no publications, continue to work */ + } else { + if (errbuf) + snprintf(errbuf, errsize, "%s", PQerrorMessage(connection)); + goto cleanup; + } + CLEARPGRES(res); + } /* Disable statement timeout. */ command("SET statement_timeout = 0", 0, NULL); From 1601b16bfc273044572dc738d75685932b27511f Mon Sep 17 00:00:00 2001 From: melkij Date: Fri, 20 Mar 2026 11:21:46 +0300 Subject: [PATCH 4/4] I forgot to add the option in the documentation also fix a typo --- bin/pg_repack.c | 2 +- doc/pg_repack.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 96ed1da8..1c930d10 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -293,7 +293,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', 5, "no-error-on-publicationn", &no_error_on_publication }, + { 'b', 5, "no-error-on-publication", &no_error_on_publication }, { '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 }, diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index a416e554..398f782e 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -126,6 +126,7 @@ Options: -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-publication repack even though publication for all tables is found --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