From 1ae46b5b14c4f91facba6036f02c29b2dbd9d902 Mon Sep 17 00:00:00 2001 From: Shlok Kyal Date: Wed, 10 Jul 2024 16:03:33 +0530 Subject: [PATCH v17 3/4] 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/ref/create_subscription.sgml | 4 +- src/backend/catalog/pg_publication.c | 12 ++++++ src/backend/replication/logical/proto.c | 40 +++++++++++++++---- src/backend/replication/logical/tablesync.c | 21 ++++++++-- src/backend/replication/pgoutput/pgoutput.c | 5 ++- src/test/subscription/t/011_generated.pl | 28 ++++++------- 9 files changed, 98 insertions(+), 32 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/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..1809e140ea 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -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 7405eb3deb..1c35fb6cff 100644 --- a/src/backend/replication/logical/proto.c +++ b/src/backend/replication/logical/proto.c @@ -793,8 +793,14 @@ logicalrep_write_tuple(StringInfo out, Relation rel, TupleTableSlot *slot, if (att->attisdropped) continue; - if (att->attgenerated && !include_generated_columns) - continue; + if (att->attgenerated) + { + if (!include_generated_columns) + continue; + + if (att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + } if (!column_in_column_list(att->attnum, columns)) continue; @@ -817,8 +823,14 @@ logicalrep_write_tuple(StringInfo out, Relation rel, TupleTableSlot *slot, if (att->attisdropped) continue; - if (att->attgenerated && !include_generated_columns) - continue; + if (att->attgenerated) + { + if (!include_generated_columns) + continue; + + if (att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + } if (!column_in_column_list(att->attnum, columns)) continue; @@ -957,8 +969,14 @@ logicalrep_write_attrs(StringInfo out, Relation rel, Bitmapset *columns, if (att->attisdropped) continue; - if (att->attgenerated && !include_generated_columns) - continue; + if (att->attgenerated) + { + if (!include_generated_columns) + continue; + + if (att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + } if (!column_in_column_list(att->attnum, columns)) continue; @@ -981,8 +999,14 @@ logicalrep_write_attrs(StringInfo out, Relation rel, Bitmapset *columns, if (att->attisdropped) continue; - if (att->attgenerated && !include_generated_columns) - continue; + if (att->attgenerated) + { + if (!include_generated_columns) + continue; + + if (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..c2a7d18774 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, @@ -1008,11 +1008,24 @@ fetch_remote_table_info(char *nspname, char *relname, bool **remotegenlist_res, " LEFT JOIN pg_catalog.pg_index i" " ON (i.indexrelid = pg_get_replica_identity_index(%u))" " WHERE a.attnum > 0::pg_catalog.int2" - " AND NOT a.attisdropped", lrel->remoteid); + " 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 6bc9f9d403..944554d5d7 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -786,7 +786,7 @@ send_relation_and_attrs(Relation relation, TransactionId xid, if (att->attisdropped) continue; - if (att->attgenerated && !include_generated_columns) + if (att->attgenerated && (att->attgenerated != ATTRIBUTE_GENERATED_STORED || !include_generated_columns)) continue; if (att->atttypid < FirstGenbkiObjectId) @@ -1108,6 +1108,9 @@ pgoutput_column_list_init(PGOutputData *data, List *publications, if (att->attisdropped) continue; + if (att->attgenerated && att->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + nliveatts++; } diff --git a/src/test/subscription/t/011_generated.pl b/src/test/subscription/t/011_generated.pl index 9e26373c43..1c49f4172c 100644 --- a/src/test/subscription/t/011_generated.pl +++ b/src/test/subscription/t/011_generated.pl @@ -30,22 +30,22 @@ $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 -# subscriber-side tab3 has DIFFERENT COMPUTATION generated col 'b'. +# publisher-side tab3 has stored generated col 'b' but +# subscriber-side tab3 has DIFFERENT COMPUTATION stored generated col 'b'. $node_publisher->safe_psql('postgres', "CREATE TABLE tab3 (a int, b int GENERATED ALWAYS AS (a + 10) STORED)"); $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 -# subscriber-side tab4 has non-generated col 'b', and generated-col 'c' +# publisher-side tab4 has stored generated cols 'b' and 'c' but +# subscriber-side tab4 has non-generated col 'b', and stored generated-col 'c' # where columns on publisher/subscriber are in a different order $node_publisher->safe_psql('postgres', "CREATE TABLE tab4 (a int, b int GENERATED ALWAYS AS (a * 2) STORED, c int GENERATED ALWAYS AS (a * 2) STORED)" @@ -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,8 +232,8 @@ 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. -# This changes the generated column into a non-generated column. +# TEST tab6: Drop the stored generated column's expression on subscriber side. +# This changes the stored generated column into a non-generated column. # $node_subscriber->safe_psql('postgres', "ALTER TABLE tab6 ALTER COLUMN c DROP EXPRESSION"); -- 2.41.0.windows.3