From 72488e973b1800570f54288250afceb3ee174e4a Mon Sep 17 00:00:00 2001 From: Shlok Kyal Date: Tue, 16 Jul 2024 14:07:00 +0530 Subject: [PATCH v18 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/protocol.sgml | 7 ++-- 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 | 8 +++- src/backend/replication/pgoutput/pgoutput.c | 13 +++++- src/test/subscription/t/011_generated.pl | 34 ++++++++-------- 10 files changed, 102 insertions(+), 36 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..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 e694baca0a..cad1b76e7a 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 935be7f934..b1407cc97d 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -1014,7 +1014,13 @@ fetch_remote_table_info(char *nspname, char *relname, bool **remotegenlist_res, { bool gencols_allowed = server_version >= 180000 && MySubscription->includegencols; - if(!gencols_allowed) + 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 = ''"); diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 6bc9f9d403..a256ab7262 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -786,8 +786,14 @@ send_relation_and_attrs(Relation relation, TransactionId xid, 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 (att->atttypid < FirstGenbkiObjectId) continue; @@ -1108,6 +1114,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 d4327717cc..72c0d061c2 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)"); @@ -71,7 +71,7 @@ $node_subscriber->safe_psql('postgres', ); # tab7: -# publisher-side tab7 has generated col 'b' but +# publisher-side tab7 has stored generated col 'b' but # subscriber-side tab7 do not have col 'b' $node_publisher->safe_psql('postgres', "CREATE TABLE tab7 (a int, b int GENERATED ALWAYS AS (a * 2) STORED)" @@ -167,7 +167,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)"); @@ -183,8 +183,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'). # @@ -201,8 +201,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)"); @@ -218,7 +218,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. # @@ -246,8 +246,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"); @@ -262,7 +262,7 @@ is( $result, qq(1|2|22 5|10|10), 'after drop generated column expression'); # -# TEST tab7: publisher-side col 'b' is generated and subscriber-side do not have col 'b' and +# TEST tab7: publisher-side col 'b' is stored generated and subscriber-side do not have col 'b' and # 'include_generated_column' is 'true' so confirmed that col 'b' IS NOT replicated and # it will throw an error. The subscription sub7 is created here, instead of earlier with the # other subscriptions, because sub7 will cause the tablesync worker to restart repetitively. @@ -276,7 +276,7 @@ $node_subscriber->wait_for_log( $node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub7"); # -# TEST tab7: publisher-side col 'b' is generated and subscriber-side do not have col 'b' and +# TEST tab7: publisher-side col 'b' is stored generated and subscriber-side do not have col 'b' and # 'include_generated_column' is 'false' so confirmed that col 'b' IS NOT replicated. # $node_subscriber->safe_psql('postgres', -- 2.34.1