From c70ef2dec9c792b579c205d558a4dbf1f98f1c5f Mon Sep 17 00:00:00 2001 From: Shlok Kyal Date: Mon, 8 Jul 2024 16:19:46 +0530 Subject: [PATCH v16 3/3] Fix behaviour for Virtual Generated columns Currently during tablesync Virtual generated columns are also replicated. Also during decoding a 'null' value appears for virtual generated column. We are not supporting replication of virtual generated columns for now. This patch fixes the behaviour for the same. This patch has a dependency on Virtual Generated Columns https://www.postgresql.org/message-id/flat/787a962749e7a822a44803ffbbdf021d8573ff53.camel%40post.pl#b64569231c9e1768e07f6bdc36c4070b --- .../expected/generated_columns.out | 1 + .../test_decoding/sql/generated_columns.sql | 4 +++- contrib/test_decoding/test_decoding.c | 15 +++++++++++-- doc/src/sgml/protocol.sgml | 7 +++--- doc/src/sgml/ref/create_subscription.sgml | 4 ++-- src/backend/catalog/pg_publication.c | 18 ++++++++++++--- src/backend/replication/logical/proto.c | 12 ++++++++++ src/backend/replication/logical/tablesync.c | 19 +++++++++++++--- src/backend/replication/pgoutput/pgoutput.c | 12 ++++++++++ src/test/subscription/t/011_generated.pl | 22 +++++++++---------- 10 files changed, 89 insertions(+), 25 deletions(-) diff --git a/contrib/test_decoding/expected/generated_columns.out b/contrib/test_decoding/expected/generated_columns.out index f3b26aa9e1..a79510705c 100644 --- a/contrib/test_decoding/expected/generated_columns.out +++ b/contrib/test_decoding/expected/generated_columns.out @@ -50,3 +50,4 @@ SELECT 'stop' FROM pg_drop_replication_slot('regression_slot'); stop (1 row) +-- TODO: Add tests related to decoding of VIRTUAL GENERATED columns diff --git a/contrib/test_decoding/sql/generated_columns.sql b/contrib/test_decoding/sql/generated_columns.sql index 6d6d1d6564..997cdebc7e 100644 --- a/contrib/test_decoding/sql/generated_columns.sql +++ b/contrib/test_decoding/sql/generated_columns.sql @@ -19,4 +19,6 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc DROP TABLE gencoltable; -SELECT 'stop' FROM pg_drop_replication_slot('regression_slot'); \ No newline at end of file +SELECT 'stop' FROM pg_drop_replication_slot('regression_slot'); + +-- TODO: Add tests related to decoding of VIRTUAL GENERATED columns \ No newline at end of file diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c index eaa3dbf9db..a847050f6e 100644 --- a/contrib/test_decoding/test_decoding.c +++ b/contrib/test_decoding/test_decoding.c @@ -557,8 +557,19 @@ tuple_to_stringinfo(StringInfo s, TupleDesc tupdesc, HeapTuple tuple, if (attr->attisdropped) continue; - if (attr->attgenerated && !include_generated_columns) - continue; + if (attr->attgenerated) + { + /* + * Don't print generated columns when + * 'include_generated_columns' is false. + */ + if (!include_generated_columns) + continue; + + /* Don't print generated columns unless they are STORED. */ + if (attr->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + } /* * Don't print system columns, oid will already have been printed if diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 226c3641b9..06554fb2af 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -3310,9 +3310,10 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" include_generated_columns - Boolean option to enable generated columns. This option controls - whether generated columns should be included in the string - representation of tuples during logical decoding in PostgreSQL. + Boolean option to enable STORED generated columns. + This option controls whether STORED generated columns + should be included in the string representation of tuples during logical + decoding in PostgreSQL. diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml index 8fb4491b65..91e33174dc 100644 --- a/doc/src/sgml/ref/create_subscription.sgml +++ b/doc/src/sgml/ref/create_subscription.sgml @@ -433,8 +433,8 @@ CREATE SUBSCRIPTION subscription_nameinclude_generated_columns (boolean) - Specifies whether the generated columns present in the tables - associated with the subscription should be replicated. + Specifies whether the STORED generated columns present + in the tables associated with the subscription should be replicated. The default is false. diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index f611148472..52b4a6ef9a 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -490,9 +490,9 @@ 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. + * to have in a publication column list (no system or virtual 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 @@ -506,6 +506,7 @@ publication_translate_columns(Relation targetrel, List *columns, Bitmapset *set = NULL; ListCell *lc; int n = 0; + TupleDesc tupdesc = RelationGetDescr(targetrel); /* Bail out when no column list defined. */ if (!columns) @@ -520,6 +521,7 @@ publication_translate_columns(Relation targetrel, List *columns, { char *colname = strVal(lfirst(lc)); AttrNumber attnum = get_attnum(RelationGetRelid(targetrel), colname); + Form_pg_attribute att; if (attnum == InvalidAttrNumber) ereport(ERROR, @@ -533,6 +535,13 @@ publication_translate_columns(Relation targetrel, List *columns, errmsg("cannot use system column \"%s\" in publication column list", colname)); + att = TupleDescAttr(tupdesc, attnum - 1); + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + ereport(ERROR, + errcode(ERRCODE_INVALID_COLUMN_REFERENCE), + errmsg("generated column \"%s\" is not STORED so cannot be used in a publication column list", + colname)); + if (bms_is_member(attnum, set)) ereport(ERROR, errcode(ERRCODE_DUPLICATE_OBJECT), @@ -1228,6 +1237,9 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) if (att->attisdropped) continue; + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + attnums[nattnums++] = att->attnum; } diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c index 24528dc41e..789914a4b5 100644 --- a/src/backend/replication/logical/proto.c +++ b/src/backend/replication/logical/proto.c @@ -784,6 +784,9 @@ logicalrep_write_tuple(StringInfo out, Relation rel, TupleTableSlot *slot, if (att->attisdropped) continue; + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + if (!column_in_column_list(att->attnum, columns)) continue; @@ -805,6 +808,9 @@ logicalrep_write_tuple(StringInfo out, Relation rel, TupleTableSlot *slot, if (att->attisdropped) continue; + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + if (!column_in_column_list(att->attnum, columns)) continue; @@ -941,6 +947,9 @@ logicalrep_write_attrs(StringInfo out, Relation rel, Bitmapset *columns) if (att->attisdropped) continue; + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + if (!column_in_column_list(att->attnum, columns)) continue; @@ -962,6 +971,9 @@ logicalrep_write_attrs(StringInfo out, Relation rel, Bitmapset *columns) if (att->attisdropped) continue; + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + if (!column_in_column_list(att->attnum, columns)) continue; diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index 1edba12a36..52887690a3 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -714,7 +714,7 @@ make_copy_attnamelist(LogicalRepRelMapEntry *rel, bool *remotegenlist) int remote_attnum; Form_pg_attribute attr = TupleDescAttr(desc, i); - if (!attr->attgenerated) + if (attr->attgenerated != ATTRIBUTE_GENERATED_STORED) continue; remote_attnum = logicalrep_rel_att_by_name(&rel->remoterel, @@ -1010,9 +1010,22 @@ fetch_remote_table_info(char *nspname, char *relname, bool **remotegenlist_res, " WHERE a.attnum > 0::pg_catalog.int2" " AND NOT a.attisdropped", lrel->remoteid); - if ((server_version >= 120000 && server_version < 180000) || - !MySubscription->includegencols) + if(server_version >= 120000) + { + bool gencols_allowed = server_version >= 180000 && MySubscription->includegencols; + + if (gencols_allowed) + { + /* Replication of generated cols is supported, but not VIRTUAL cols. */ + /* TODO: use ATTRIBUTE_GENERATED_VIRTUAL*/ + appendStringInfo(&cmd, " AND a.attgenerated != 'v'"); + } + else + { + /* Replication of generated cols is not supported. */ appendStringInfo(&cmd, " AND a.attgenerated = ''"); + } + } appendStringInfo(&cmd, " AND a.attrelid = %u" diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 5ff5078bbc..b7fd4c3882 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -783,6 +783,9 @@ send_relation_and_attrs(Relation relation, TransactionId xid, if (att->attisdropped) continue; + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + if (att->atttypid < FirstGenbkiObjectId) continue; @@ -1042,6 +1045,9 @@ prepare_all_columns_bms(PGOutputData *data, RelationSyncEntry *entry, if (att->attisdropped) continue; + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + cols = bms_add_member(cols, i + 1); } @@ -1130,6 +1136,9 @@ pgoutput_column_list_init(PGOutputData *data, List *publications, if (att->attisdropped) continue; + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + nliveatts++; } @@ -1163,6 +1172,9 @@ pgoutput_column_list_init(PGOutputData *data, List *publications, if (att->attisdropped) continue; + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + /* Count all valid attributes */ nliveatts++; diff --git a/src/test/subscription/t/011_generated.pl b/src/test/subscription/t/011_generated.pl index 9e26373c43..8208fd93bc 100644 --- a/src/test/subscription/t/011_generated.pl +++ b/src/test/subscription/t/011_generated.pl @@ -30,13 +30,13 @@ $node_subscriber->safe_psql('postgres', "CREATE TABLE tab1 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 22) STORED, c int)" ); -# publisher-side tab2 has generated col 'b' but subscriber-side tab2 has NON-generated col 'b'. +# publisher-side tab2 has STORED generated col 'b' but subscriber-side tab2 has NON-generated col 'b'. $node_publisher->safe_psql('postgres', "CREATE TABLE tab2 (a int, b int GENERATED ALWAYS AS (a * 2) STORED)"); $node_subscriber->safe_psql('postgres', "CREATE TABLE tab2 (a int, b int)"); # tab3: -# publisher-side tab3 has generated col 'b' but +# publisher-side tab3 has STORED generated col 'b' but # subscriber-side tab3 has DIFFERENT COMPUTATION generated col 'b'. $node_publisher->safe_psql('postgres', "CREATE TABLE tab3 (a int, b int GENERATED ALWAYS AS (a + 10) STORED)"); @@ -44,7 +44,7 @@ $node_subscriber->safe_psql('postgres', "CREATE TABLE tab3 (a int, b int GENERATED ALWAYS AS (a + 20) STORED)"); # tab4: -# publisher-side tab4 has generated cols 'b' and 'c' but +# publisher-side tab4 has STORED generated cols 'b' and 'c' but # subscriber-side tab4 has non-generated col 'b', and generated-col 'c' # where columns on publisher/subscriber are in a different order $node_publisher->safe_psql('postgres', @@ -56,7 +56,7 @@ $node_subscriber->safe_psql('postgres', # tab5: # publisher-side tab5 has non-generated col 'b' but -# subscriber-side tab5 has generated col 'b' +# subscriber-side tab5 has STORED generated col 'b' $node_publisher->safe_psql('postgres', "CREATE TABLE tab5 (a int, b int)"); $node_subscriber->safe_psql('postgres', "CREATE TABLE tab5 (a int, b int GENERATED ALWAYS AS (a * 22) STORED)"); @@ -153,7 +153,7 @@ is( $result, qq(1|22| 6|132|), 'generated columns replicated'); # -# TEST tab2: the publisher-side col 'b' is generated, and the subscriber-side +# TEST tab2: the publisher-side col 'b' is STORED generated, and the subscriber-side # col 'b' is not generated, so confirm that col 'b' IS replicated. # $node_publisher->safe_psql('postgres', "INSERT INTO tab2 VALUES (4), (5)"); @@ -169,8 +169,8 @@ is( $result, qq(1|2 ); # -# TEST tab3: the publisher-side col 'b' is generated, and the subscriber-side -# col 'b' is also generated, so confirmed that col 'b' IS NOT replicated. We +# TEST tab3: the publisher-side col 'b' is STORED generated, and the subscriber-side +# col 'b' is also STORED generated, so confirmed that col 'b' IS NOT replicated. We # can know this because the result value is the subscriber-side computation # (which is not the same as the publisher-side computation for col 'b'). # @@ -187,8 +187,8 @@ is( $result, qq(1|21 ); # -# TEST tab4: the publisher-side cols 'b' and 'c' are generated and subscriber-side -# col 'b' is not generated and col 'c' is generated. So confirmed that the different +# TEST tab4: the publisher-side cols 'b' and 'c' are STORED generated and subscriber-side +# col 'b' is not generated and col 'c' is STORED generated. So confirmed that the different # order of columns on subscriber-side replicate data to correct columns. # $node_publisher->safe_psql('postgres', "INSERT INTO tab4 VALUES (4), (5)"); @@ -204,7 +204,7 @@ is( $result, qq(1|2|22 # # TEST tab5: publisher-side col 'b' is not-generated and subscriber-side col 'b' -# is generated, so confirmed that col 'b' IS NOT replicated and it will throw an error. +# is STORED generated, so confirmed that col 'b' IS NOT replicated and it will throw an error. # The subscription sub5 is created here, instead of earlier with the other subscriptions, # because sub5 will cause the tablesync worker to restart repetitively. # @@ -232,7 +232,7 @@ is( $result, qq(1|2|22 3|6|66), 'add new table to existing publication'); # -# TEST tab6: Drop the generated column's expression on subscriber side. +# TEST tab6: Drop the STORED generated column's expression on subscriber side. # This changes the generated column into a non-generated column. # $node_subscriber->safe_psql('postgres', -- 2.34.1