From ae317c425d0d4157addedbeff3aa56c214090214 Mon Sep 17 00:00:00 2001 From: Mehmet Yilmaz Date: Fri, 28 Nov 2025 10:32:51 +0000 Subject: [PATCH] review update --- .../distributed/commands/publication.c | 30 ++--- src/test/regress/expected/pg18.out | 109 ++++++++++++++++++ src/test/regress/sql/pg18.sql | 94 ++++++++++++++- 3 files changed, 217 insertions(+), 16 deletions(-) diff --git a/src/backend/distributed/commands/publication.c b/src/backend/distributed/commands/publication.c index 0e48738f0..7c1db06d6 100644 --- a/src/backend/distributed/commands/publication.c +++ b/src/backend/distributed/commands/publication.c @@ -198,22 +198,22 @@ BuildCreatePublicationStmt(Oid publicationId) /* WITH (publish_generated_columns = ...) option (PG18+) */ #if PG_VERSION_NUM >= PG_VERSION_18 - if (publicationForm->pubgencols == 's') /* stored */ - { - DefElem *pubGenColsOption = - makeDefElem("publish_generated_columns", - (Node *) makeString("stored"), - -1); + if (publicationForm->pubgencols == 's') /* stored */ + { + DefElem *pubGenColsOption = + makeDefElem("publish_generated_columns", + (Node *) makeString("stored"), + -1); - createPubStmt->options = - lappend(createPubStmt->options, pubGenColsOption); - } - else if (publicationForm->pubgencols != 'n') /* 'n' = none (default) */ - { - ereport(ERROR, - (errmsg("unexpected pubgencols value '%c' for publication %u", - publicationForm->pubgencols, publicationId))); - } + createPubStmt->options = + lappend(createPubStmt->options, pubGenColsOption); + } + else if (publicationForm->pubgencols != 'n') /* 'n' = none (default) */ + { + ereport(ERROR, + (errmsg("unexpected pubgencols value '%c' for publication %u", + publicationForm->pubgencols, publicationId))); + } #endif diff --git a/src/test/regress/expected/pg18.out b/src/test/regress/expected/pg18.out index 352d1cfb1..08b6c46e3 100644 --- a/src/test/regress/expected/pg18.out +++ b/src/test/regress/expected/pg18.out @@ -1133,6 +1133,115 @@ ORDER BY pubname; pub_gen_cols_stored | s (2 rows) +-- Now verify ALTER PUBLICATION .. SET (publish_generated_columns = none) +-- propagates to workers as well. +\c - - - :master_port +SET search_path TO pg18_publication; +ALTER PUBLICATION pub_gen_cols_stored + SET (publish_generated_columns = none); +-- coordinator: both publications should now have pubgencols = 'n' +SELECT pubname, pubgencols +FROM pg_publication +WHERE pubname IN ('pub_gen_cols_stored', 'pub_gen_cols_none') +ORDER BY pubname; + pubname | pubgencols +--------------------------------------------------------------------- + pub_gen_cols_none | n + pub_gen_cols_stored | n +(2 rows) + +-- worker 1: pubgencols must match coordinator +\c - - - :worker_1_port +SET search_path TO pg18_publication; +SELECT pubname, pubgencols +FROM pg_publication +WHERE pubname IN ('pub_gen_cols_stored', 'pub_gen_cols_none') +ORDER BY pubname; + pubname | pubgencols +--------------------------------------------------------------------- + pub_gen_cols_none | n + pub_gen_cols_stored | n +(2 rows) + +-- worker 2: same check +\c - - - :worker_2_port +SET search_path TO pg18_publication; +SELECT pubname, pubgencols +FROM pg_publication +WHERE pubname IN ('pub_gen_cols_stored', 'pub_gen_cols_none') +ORDER BY pubname; + pubname | pubgencols +--------------------------------------------------------------------- + pub_gen_cols_none | n + pub_gen_cols_stored | n +(2 rows) + +-- Column list precedence test: Citus must preserve both prattrs and pubgencols +\c - - - :master_port +SET search_path TO pg18_publication; +-- Case 1: column list explicitly includes the generated column, flag = none +CREATE PUBLICATION pub_gen_cols_list_includes_b + FOR TABLE gen_pub_tab (id, a, b) + WITH (publish_generated_columns = none); +-- Case 2: column list excludes the generated column, flag = stored +CREATE PUBLICATION pub_gen_cols_list_excludes_b + FOR TABLE gen_pub_tab (id, a) + WITH (publish_generated_columns = stored); +-- Helper: show pubname, pubgencols, and column list (prattrs) for gen_pub_tab +SELECT p.pubname, + p.pubgencols, + r.prattrs +FROM pg_publication p +JOIN pg_publication_rel r ON p.oid = r.prpubid +JOIN pg_class c ON c.oid = r.prrelid +WHERE p.pubname IN ('pub_gen_cols_list_includes_b', + 'pub_gen_cols_list_excludes_b') + AND c.relname = 'gen_pub_tab' +ORDER BY p.pubname; + pubname | pubgencols | prattrs +--------------------------------------------------------------------- + pub_gen_cols_list_excludes_b | s | 1 2 + pub_gen_cols_list_includes_b | n | 1 2 3 +(2 rows) + +-- worker 1: must see the same pubgencols + prattrs +\c - - - :worker_1_port +SET search_path TO pg18_publication; +SELECT p.pubname, + p.pubgencols, + r.prattrs +FROM pg_publication p +JOIN pg_publication_rel r ON p.oid = r.prpubid +JOIN pg_class c ON c.oid = r.prrelid +WHERE p.pubname IN ('pub_gen_cols_list_includes_b', + 'pub_gen_cols_list_excludes_b') + AND c.relname = 'gen_pub_tab' +ORDER BY p.pubname; + pubname | pubgencols | prattrs +--------------------------------------------------------------------- + pub_gen_cols_list_excludes_b | s | 1 2 + pub_gen_cols_list_includes_b | n | 1 2 3 +(2 rows) + +-- worker 2: same check +\c - - - :worker_2_port +SET search_path TO pg18_publication; +SELECT p.pubname, + p.pubgencols, + r.prattrs +FROM pg_publication p +JOIN pg_publication_rel r ON p.oid = r.prpubid +JOIN pg_class c ON c.oid = r.prrelid +WHERE p.pubname IN ('pub_gen_cols_list_includes_b', + 'pub_gen_cols_list_excludes_b') + AND c.relname = 'gen_pub_tab' +ORDER BY p.pubname; + pubname | pubgencols | prattrs +--------------------------------------------------------------------- + pub_gen_cols_list_excludes_b | s | 1 2 + pub_gen_cols_list_includes_b | n | 1 2 3 +(2 rows) + -- back to coordinator for subsequent tests / cleanup \c - - - :master_port SET search_path TO pg18_publication; diff --git a/src/test/regress/sql/pg18.sql b/src/test/regress/sql/pg18.sql index d84a5dbaa..df9f71869 100644 --- a/src/test/regress/sql/pg18.sql +++ b/src/test/regress/sql/pg18.sql @@ -682,14 +682,106 @@ FROM pg_publication WHERE pubname IN ('pub_gen_cols_stored', 'pub_gen_cols_none') ORDER BY pubname; +-- Now verify ALTER PUBLICATION .. SET (publish_generated_columns = none) +-- propagates to workers as well. + +\c - - - :master_port +SET search_path TO pg18_publication; + +ALTER PUBLICATION pub_gen_cols_stored + SET (publish_generated_columns = none); + +-- coordinator: both publications should now have pubgencols = 'n' +SELECT pubname, pubgencols +FROM pg_publication +WHERE pubname IN ('pub_gen_cols_stored', 'pub_gen_cols_none') +ORDER BY pubname; + +-- worker 1: pubgencols must match coordinator +\c - - - :worker_1_port +SET search_path TO pg18_publication; + +SELECT pubname, pubgencols +FROM pg_publication +WHERE pubname IN ('pub_gen_cols_stored', 'pub_gen_cols_none') +ORDER BY pubname; + +-- worker 2: same check +\c - - - :worker_2_port +SET search_path TO pg18_publication; + +SELECT pubname, pubgencols +FROM pg_publication +WHERE pubname IN ('pub_gen_cols_stored', 'pub_gen_cols_none') +ORDER BY pubname; + +-- Column list precedence test: Citus must preserve both prattrs and pubgencols + +\c - - - :master_port +SET search_path TO pg18_publication; + +-- Case 1: column list explicitly includes the generated column, flag = none +CREATE PUBLICATION pub_gen_cols_list_includes_b + FOR TABLE gen_pub_tab (id, a, b) + WITH (publish_generated_columns = none); + +-- Case 2: column list excludes the generated column, flag = stored +CREATE PUBLICATION pub_gen_cols_list_excludes_b + FOR TABLE gen_pub_tab (id, a) + WITH (publish_generated_columns = stored); + +-- Helper: show pubname, pubgencols, and column list (prattrs) for gen_pub_tab +SELECT p.pubname, + p.pubgencols, + r.prattrs +FROM pg_publication p +JOIN pg_publication_rel r ON p.oid = r.prpubid +JOIN pg_class c ON c.oid = r.prrelid +WHERE p.pubname IN ('pub_gen_cols_list_includes_b', + 'pub_gen_cols_list_excludes_b') + AND c.relname = 'gen_pub_tab' +ORDER BY p.pubname; + +-- worker 1: must see the same pubgencols + prattrs +\c - - - :worker_1_port +SET search_path TO pg18_publication; + +SELECT p.pubname, + p.pubgencols, + r.prattrs +FROM pg_publication p +JOIN pg_publication_rel r ON p.oid = r.prpubid +JOIN pg_class c ON c.oid = r.prrelid +WHERE p.pubname IN ('pub_gen_cols_list_includes_b', + 'pub_gen_cols_list_excludes_b') + AND c.relname = 'gen_pub_tab' +ORDER BY p.pubname; + +-- worker 2: same check +\c - - - :worker_2_port +SET search_path TO pg18_publication; + +SELECT p.pubname, + p.pubgencols, + r.prattrs +FROM pg_publication p +JOIN pg_publication_rel r ON p.oid = r.prpubid +JOIN pg_class c ON c.oid = r.prrelid +WHERE p.pubname IN ('pub_gen_cols_list_includes_b', + 'pub_gen_cols_list_excludes_b') + AND c.relname = 'gen_pub_tab' +ORDER BY p.pubname; + -- back to coordinator for subsequent tests / cleanup \c - - - :master_port SET search_path TO pg18_publication; DROP PUBLICATION pub_gen_cols_stored; DROP PUBLICATION pub_gen_cols_none; +DROP PUBLICATION pub_gen_cols_list_includes_b; +DROP PUBLICATION pub_gen_cols_list_excludes_b; DROP SCHEMA pg18_publication CASCADE; SET search_path TO pg18_nn; --- PG18: verify publish_generated_columns is preserved for distributed tables +-- END: PG18: verify publish_generated_columns is preserved for distributed tables -- cleanup with minimum verbosity SET client_min_messages TO ERROR;