From 4d7f8d96486600561d4be0afd96c8b92c63bf41a Mon Sep 17 00:00:00 2001 From: Shlok Kyal Date: Fri, 19 Jul 2024 11:00:14 +0530 Subject: [PATCH v20 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 | 18 +++++++-- 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 | 36 ++++++++--------- 10 files changed, 106 insertions(+), 40 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..71466b1583 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 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 2e90d42bdc..14a0aae416 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 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 4537c6c5da..b7dbe6117f 100644 --- a/src/test/subscription/t/011_generated.pl +++ b/src/test/subscription/t/011_generated.pl @@ -31,23 +31,23 @@ $node_subscriber->safe_psql('postgres', ); # tab2: -# publisher-side tab2 has generated col 'b'. +# publisher-side tab2 has stored generated col 'b'. # 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'. -# subscriber-side tab3 has generated col 'b', using a different computation. +# publisher-side tab3 has stored generated col 'b'. +# subscriber-side tab3 has stored generated col 'b', using a different computation. $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'. -# Subscriber-side tab4 has non-generated col 'b', and generated-col 'c'. +# Publisher-side tab4 has stored generated cols 'b' and 'c'. +# Subscriber-side tab4 has non-generated col 'b', and stored generated-col 'c'. # 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)" @@ -58,7 +58,7 @@ $node_subscriber->safe_psql('postgres', # tab5: # Publisher-side tab5 has non-generated col 'b'. -# 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)"); @@ -73,7 +73,7 @@ $node_subscriber->safe_psql('postgres', ); # tab7: -# Publisher-side tab7 has generated col 'b'. +# Publisher-side tab7 has stored generated col 'b'. # Subscriber-side tab7 does not have any col 'b'. $node_publisher->safe_psql('postgres', "CREATE TABLE tab7 (a int, b int GENERATED ALWAYS AS (a * 2) STORED)" @@ -170,7 +170,7 @@ is( $result, qq(1|22| # # TEST tab2: -# publisher-side tab2 has generated col 'b'. +# publisher-side tab2 has stored generated col 'b'. # subscriber-side tab2 has non-generated col 'b'. # # Confirm that col 'b' is replicated. @@ -189,8 +189,8 @@ is( $result, qq(1|2 # # TEST tab3: -# publisher-side tab3 has generated col 'b'. -# subscriber-side tab3 has generated col 'b', using a different computation. +# publisher-side tab3 has stored generated col 'b'. +# subscriber-side tab3 has stored generated col 'b', using a different computation. # # Confirm that col 'b' is NOT replicated. We can know this because the result # value is the subscriber-side computation (which is different from the @@ -210,8 +210,8 @@ is( $result, qq(1|21 # # TEST tab4: -# Publisher-side tab4 has generated cols 'b' and 'c'. -# Subscriber-side tab4 has non-generated col 'b', and generated-col 'c'. +# Publisher-side tab4 has stored generated cols 'b' and 'c'. +# Subscriber-side tab4 has non-generated col 'b', and stored generated-col 'c'. # Columns on publisher/subscriber are in a different order. # # Confirm despite the different order columns, they still replicate correctly. @@ -230,7 +230,7 @@ is( $result, qq(1|2|22 # # TEST tab5: # Publisher-side tab5 has non-generated col 'b'. -# Subscriber-side tab5 has generated col 'b'. +# Subscriber-side tab5 has stored generated col 'b'. # # Confirm that col 'b' is not replicated and it will throw an error. # @@ -263,11 +263,11 @@ is( $result, qq(1|2|22 # # TEST tab6: -# Drop the generated column's expression on subscriber side. -# This changes the generated column into a non-generated column. +# Drop the stored generated column's expression on subscriber side. +# This changes the stored generated column into a non-generated column. # # Confirm that replication happens after the drop expression, because now we -# are replicating from a generated column to a non-generated column. +# are replicating from a stored generated column to a non-generated column. # $node_subscriber->safe_psql('postgres', "ALTER TABLE tab6 ALTER COLUMN c DROP EXPRESSION"); @@ -283,7 +283,7 @@ is( $result, qq(1|2|22 # # TEST tab7, false -# Publisher-side tab7 has generated col 'b'. +# Publisher-side tab7 has stored generated col 'b'. # Subscriber-side tab7 does not have any col 'b'. # 'include_generated_columns' is true. # @@ -303,7 +303,7 @@ $node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub7"); # # TEST tab7: -# Publisher-side tab7 has generated col 'b'. +# Publisher-side tab7 has stored generated col 'b'. # Subscriber-side tab7 does not have any col 'b'. # 'include_generated_columns' is default (false). # -- 2.34.1