From 34e14aa0b0be5ee8b98870817b13f320ee00d4b0 Mon Sep 17 00:00:00 2001 From: Khanna Date: Wed, 24 Jul 2024 12:16:45 +0530 Subject: [PATCH v21 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 | 28 ++++++------- 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 3320c25a60..a2713a95b1 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -3326,9 +3326,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 4624649cd7..c02de23743 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -784,8 +784,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; @@ -1106,6 +1112,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 8ff3f4ad05..b61bbdd137 100644 --- a/src/test/subscription/t/011_generated.pl +++ b/src/test/subscription/t/011_generated.pl @@ -38,8 +38,8 @@ $node_subscriber2->safe_psql('postgres', "CREATE TABLE tab1 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 22) STORED, c int)" ); -# publisher-side has generated col 'b'. -# subscriber-side has generated col 'b', with different computation. +# publisher-side has stored generated col 'b'. +# subscriber-side has stored generated col 'b', with different computation. $node_publisher->safe_psql('postgres', "CREATE TABLE tab_gen_to_gen (a int, b int GENERATED ALWAYS AS (a + 10) STORED)"); $node_subscriber->safe_psql('postgres', @@ -47,14 +47,14 @@ $node_subscriber->safe_psql('postgres', $node_subscriber2->safe_psql('postgres', "CREATE TABLE tab_gen_to_gen (a int, b int GENERATED ALWAYS AS (a + 20) STORED)"); -# publisher-side has generated col 'b'. +# publisher-side has stored generated col 'b'. # subscriber-side has non-generated col 'b'. $node_publisher->safe_psql('postgres', "CREATE TABLE tab_gen_to_nogen (a int, b int GENERATED ALWAYS AS (a * 2) STORED)"); $node_subscriber->safe_psql('postgres', "CREATE TABLE tab_gen_to_nogen (a int, b int)"); $node_subscriber2->safe_psql('postgres', "CREATE TABLE tab_gen_to_nogen (a int, b int)"); -# publisher-side has generated col 'b'. +# publisher-side has stored generated col 'b'. # subscriber-side col 'b' is missing. $node_publisher->safe_psql('postgres', "CREATE TABLE tab_gen_to_missing (a int, b int GENERATED ALWAYS AS (a * 2) STORED)" @@ -67,7 +67,7 @@ $node_subscriber2->safe_psql('postgres', ); # publisher-side has non-generated col 'b'. -# subscriber-side has generated col 'b'. +# subscriber-side has stored generated col 'b'. $node_publisher->safe_psql('postgres', "CREATE TABLE tab_nogen_to_gen (a int, b int)"); $node_subscriber->safe_psql('postgres', "CREATE TABLE tab_nogen_to_gen (a int, b int GENERATED ALWAYS AS (a * 22) STORED)"); @@ -87,8 +87,8 @@ $node_subscriber2->safe_psql('postgres', ); # tab_order: -# publisher-side has generated cols 'b' and 'c' but -# subscriber-side has non-generated col 'b', and generated-col 'c' +# publisher-side has stored generated cols 'b' and 'c' but +# subscriber-side 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 tab_order (a int, b int GENERATED ALWAYS AS (a * 2) STORED, c int GENERATED ALWAYS AS (a * 2) STORED)" @@ -212,7 +212,7 @@ is( $result, qq(1|22| ##################### # TEST tab_gen_to_gen # -# publisher-side has generated col 'b'. +# publisher-side has stored generated col 'b'. # subscriber-side has generated col 'b', using a different computation. ##################### @@ -250,7 +250,7 @@ is( $result, qq(1|21 ##################### # TEST tab_gen_to_nogen # -# publisher-side has generated col 'b'. +# publisher-side has stored generated col 'b'. # subscriber-side has non-generated col 'b'. ##################### @@ -286,7 +286,7 @@ is( $result, qq(1|2 ##################### # TEST tab_gen_to_missing # -# publisher-side col 'b' is generated. +# publisher-side col 'b' is stored generated. # subscriber-side col 'b' is missing ##################### @@ -384,8 +384,8 @@ is( $result, qq(1|2 ##################### # TEST tab_order: # -# publisher-side cols 'b' and 'c' are generated -# subscriber-side col 'b' is not generated and col 'c' is generated. +# publisher-side cols 'b' and 'c' are stored generated +# subscriber-side col 'b' is not generated and col 'c' is stored generated. # But pub/sub table cols are in different order. ##################### @@ -425,8 +425,8 @@ is( $result, qq(1|2|22 ##################### # TEST tabl_alter # -# 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. ##################### # change a gencol to a nogen col -- 2.41.0.windows.3