From 0f3e3786a7619650e817b6e212d865361b2b06c9 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Fri, 2 Aug 2024 12:42:17 +1000 Subject: [PATCH v3] fix new collist validation msg --- src/backend/catalog/pg_publication.c | 26 +++++++++++++++----------- src/backend/commands/publicationcmds.c | 20 +++++++++----------- src/include/catalog/pg_publication.h | 3 +++ src/test/regress/expected/publication.out | 7 +++++++ src/test/regress/sql/publication.sql | 4 ++++ 5 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 0602398..bec11ee 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -48,9 +48,6 @@ typedef struct * table. */ } published_rel; -static void publication_translate_columns(Relation targetrel, List *columns, - int *natts, AttrNumber **attrs); - /* * Check if relation can be in given publication and throws appropriate * error if not. @@ -401,7 +398,7 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri, * Also build an array of attnums, for storing in the catalog. */ publication_translate_columns(pri->relation, pri->columns, - &natts, &attarray); + NULL, &natts, &attarray); /* Form a tuple. */ memset(values, 0, sizeof(values)); @@ -488,18 +485,21 @@ compare_int16(const void *a, const void *b) } /* - * Translate a list of column names to an array of attribute numbers - * and a Bitmapset with them; verify that each attribute is appropriate - * to have in a publication column list (no system or generated attributes, - * no duplicates). Additional checks with replica identity are done later; - * see pub_collist_contains_invalid_column. + * Translate a list of column names to an array of attribute numbers. + * The equivalent Bitmapset is also returned when bms_attrs is specified. + * + * Verify that each attribute is appropriate to have in a + * publication column list (no system or generated attributes, no duplicates). + * Additional checks with replica identity are done later; see + * pub_collist_contains_invalid_column. * * Note that the attribute numbers are *not* offset by * FirstLowInvalidHeapAttributeNumber; system columns are forbidden so this * is okay. */ -static void +void publication_translate_columns(Relation targetrel, List *columns, + Bitmapset **bms_attrs, int *natts, AttrNumber **attrs) { AttrNumber *attarray = NULL; @@ -508,6 +508,9 @@ publication_translate_columns(Relation targetrel, List *columns, int n = 0; TupleDesc tupdesc = RelationGetDescr(targetrel); + if (bms_attrs) + *bms_attrs = set; + /* Bail out when no column list defined. */ if (!columns) return; @@ -556,7 +559,8 @@ publication_translate_columns(Relation targetrel, List *columns, *natts = n; *attrs = attarray; - bms_free(set); + if (!bms_attrs) + bms_free(set); } /* diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 6ea7099..e7e6977 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -1176,20 +1176,18 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup, newrelid = RelationGetRelid(newpubrel->relation); /* - * If the new publication has column list, transform it to a - * bitmap too. + * If the new publication has column list, validate it + * before transforming it to a bitmap too. */ if (newpubrel->columns) { - ListCell *lc; + AttrNumber *attarray = NULL; + int natts = 0; - foreach(lc, newpubrel->columns) - { - char *colname = strVal(lfirst(lc)); - AttrNumber attnum = get_attnum(newrelid, colname); - - newcolumns = bms_add_member(newcolumns, attnum); - } + publication_translate_columns(newpubrel->relation, + newpubrel->columns, + &newcolumns, + &natts, &attarray); } /* @@ -1199,7 +1197,7 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup, * expressions also match. Same for the column list. Drop the * rest. */ - if (RelationGetRelid(newpubrel->relation) == oldrelid) + if (newrelid == oldrelid) { if (equal(oldrelwhereclause, newpubrel->whereClause) && bms_equal(oldcolumns, newcolumns)) diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h index 2f1b6ab..5f99d9d 100644 --- a/src/include/catalog/pg_publication.h +++ b/src/include/catalog/pg_publication.h @@ -157,5 +157,8 @@ extern ObjectAddress publication_add_schema(Oid pubid, Oid schemaid, extern Bitmapset *pub_collist_to_bitmapset(Bitmapset *columns, Datum pubcols, MemoryContext mcxt); +extern void publication_translate_columns(Relation targetrel, List *columns, + Bitmapset **bms_attrs, + int *natts, AttrNumber **attrs); #endif /* PG_PUBLICATION_H */ diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index 30b6371..660245e 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -693,6 +693,13 @@ ERROR: cannot use generated column "d" in publication column list -- error: system attributes "ctid" not allowed in column list ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, ctid); ERROR: cannot use system column "ctid" in publication column list +ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl1 (id, ctid); +ERROR: cannot use system column "ctid" in publication column list +-- error: duplicates not allowed in column list +ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, a); +ERROR: duplicate column "a" in publication column list +ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl5 (a, a); +ERROR: duplicate column "a" in publication column list -- ok ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c); ALTER TABLE testpub_tbl5 DROP COLUMN c; -- no dice diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql index 479d4f3..f68a5b5 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -417,6 +417,10 @@ ALTER PUBLICATION testpub_fortable DROP TABLE testpub_tbl5; ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, d); -- error: system attributes "ctid" not allowed in column list ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, ctid); +ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl1 (id, ctid); +-- error: duplicates not allowed in column list +ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, a); +ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl5 (a, a); -- ok ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c); ALTER TABLE testpub_tbl5 DROP COLUMN c; -- no dice -- 1.8.3.1