From 1806ecb17aca3f08572524199182bbfa2d2c048a Mon Sep 17 00:00:00 2001 From: Shlok Kyal Date: Mon, 24 Jun 2024 18:12:52 +0530 Subject: [PATCH v10 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/decoding_into_rel.out | 1 + .../test_decoding/sql/decoding_into_rel.sql | 2 ++ contrib/test_decoding/test_decoding.c | 8 +++++++- doc/src/sgml/protocol.sgml | 8 ++++---- doc/src/sgml/ref/create_subscription.sgml | 4 ++-- src/backend/catalog/pg_publication.c | 13 +++++++++++- src/backend/replication/logical/proto.c | 8 ++++---- src/backend/replication/logical/relation.c | 3 +++ src/backend/replication/logical/tablesync.c | 19 ++++++++++++++---- src/backend/replication/pgoutput/pgoutput.c | 5 ++++- src/test/subscription/t/011_generated.pl | 20 ++++++++++--------- 11 files changed, 65 insertions(+), 26 deletions(-) diff --git a/contrib/test_decoding/expected/decoding_into_rel.out b/contrib/test_decoding/expected/decoding_into_rel.out index 94a3741408..188042cbdc 100644 --- a/contrib/test_decoding/expected/decoding_into_rel.out +++ b/contrib/test_decoding/expected/decoding_into_rel.out @@ -142,6 +142,7 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc (5 rows) DROP TABLE gencoltable; +-- TODO: Add tests related to decoding of VIRTUAL GENERATED columns SELECT 'stop' FROM pg_drop_replication_slot('regression_slot'); ?column? ---------- diff --git a/contrib/test_decoding/sql/decoding_into_rel.sql b/contrib/test_decoding/sql/decoding_into_rel.sql index 85584531a9..84afe7fdd3 100644 --- a/contrib/test_decoding/sql/decoding_into_rel.sql +++ b/contrib/test_decoding/sql/decoding_into_rel.sql @@ -52,4 +52,6 @@ INSERT INTO gencoltable (a) VALUES (4), (5), (6); SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1', 'include-generated-columns', '0'); DROP TABLE gencoltable; +-- TODO: Add tests related to decoding of VIRTUAL GENERATED columns + SELECT 'stop' FROM pg_drop_replication_slot('regression_slot'); \ No newline at end of file diff --git a/contrib/test_decoding/test_decoding.c b/contrib/test_decoding/test_decoding.c index aa7690b58e..abce99a399 100644 --- a/contrib/test_decoding/test_decoding.c +++ b/contrib/test_decoding/test_decoding.c @@ -557,7 +557,13 @@ tuple_to_stringinfo(StringInfo s, TupleDesc tupdesc, HeapTuple tuple, if (attr->attisdropped) continue; - if (attr->attgenerated && !include_generated_columns) + /* + * Don't print virtual generated column. Don't print stored + * generated column if 'include_generated_columns' is false. + * + * TODO: can use ATTRIBUTE_GENERATED_VIRTUAL to simpilfy + */ + if (attr->attgenerated && (attr->attgenerated != ATTRIBUTE_GENERATED_STORED || !include_generated_columns)) continue; /* diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 7a5637c5f3..db20ad9b24 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -3310,10 +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. - The default is false. + 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. The default is false. diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml index 8fb4491b65..79ccb9bd71 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..e5e5aef243 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) @@ -533,6 +534,16 @@ publication_translate_columns(Relation targetrel, List *columns, errmsg("cannot use system column \"%s\" in publication column list", colname)); + /* + * TODO: simplify the expression + */ + if (TupleDescAttr(tupdesc, attnum - 1)->attgenerated && + TupleDescAttr(tupdesc, attnum - 1)->attgenerated != ATTRIBUTE_GENERATED_STORED) + ereport(ERROR, + errcode(ERRCODE_INVALID_COLUMN_REFERENCE), + errmsg("cannot use virtual generated column \"%s\" in publication column list", + colname)); + if (bms_is_member(attnum, set)) ereport(ERROR, errcode(ERRCODE_DUPLICATE_OBJECT), @@ -1225,7 +1236,7 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) { Form_pg_attribute att = TupleDescAttr(desc, i); - if (att->attisdropped) + if (att->attisdropped || (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..e82e53e384 100644 --- a/src/backend/replication/logical/proto.c +++ b/src/backend/replication/logical/proto.c @@ -793,7 +793,7 @@ logicalrep_write_tuple(StringInfo out, Relation rel, TupleTableSlot *slot, if (att->attisdropped) continue; - if (att->attgenerated && !include_generated_columns) + if (att->attgenerated && (att->attgenerated != ATTRIBUTE_GENERATED_STORED || !include_generated_columns)) continue; if (!column_in_column_list(att->attnum, columns)) @@ -817,7 +817,7 @@ logicalrep_write_tuple(StringInfo out, Relation rel, TupleTableSlot *slot, if (att->attisdropped) continue; - if (att->attgenerated && !include_generated_columns) + if (att->attgenerated && (att->attgenerated != ATTRIBUTE_GENERATED_STORED || !include_generated_columns)) continue; if (!column_in_column_list(att->attnum, columns)) @@ -957,7 +957,7 @@ logicalrep_write_attrs(StringInfo out, Relation rel, Bitmapset *columns, if (att->attisdropped) continue; - if (att->attgenerated && !include_generated_columns) + if (att->attgenerated && (att->attgenerated != ATTRIBUTE_GENERATED_STORED || !include_generated_columns)) continue; if (!column_in_column_list(att->attnum, columns)) @@ -981,7 +981,7 @@ logicalrep_write_attrs(StringInfo out, Relation rel, Bitmapset *columns, if (att->attisdropped) continue; - if (att->attgenerated && !include_generated_columns) + if (att->attgenerated && (att->attgenerated != ATTRIBUTE_GENERATED_STORED || !include_generated_columns)) continue; if (!column_in_column_list(att->attnum, columns)) diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index 27c34059af..e1b1693700 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -427,6 +427,9 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) continue; } + if (attr->attgenerated && attr->attgenerated != ATTRIBUTE_GENERATED_STORED) + continue; + /* * In case 'include_generated_columns' is 'false', we should skip the * check of missing attrs for generated columns. diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index b3fde6afb3..d44f10901e 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -712,7 +712,7 @@ make_copy_attnamelist(LogicalRepRelMapEntry *rel, bool *remotegenlist) int attnum; Form_pg_attribute attr = TupleDescAttr(desc, i); - if (!attr->attgenerated) + if (attr->attgenerated != ATTRIBUTE_GENERATED_STORED) continue; attnum = logicalrep_rel_att_by_name(&rel->remoterel, @@ -1001,10 +1001,21 @@ fetch_remote_table_info(char *nspname, char *relname, bool **remotegenlist, " WHERE a.attnum > 0::pg_catalog.int2" " AND NOT a.attisdropped", lrel->remoteid); - if ((walrcv_server_version(LogRepWorkerWalRcvConn) >= 120000 && - walrcv_server_version(LogRepWorkerWalRcvConn) <= 160000) || - !MySubscription->includegencols) + if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 120000) + { + bool gencols_allowed = walrcv_server_version(LogRepWorkerWalRcvConn) >= 170000 + && MySubscription->includegencols; + if (gencols_allowed) + { + /* Replication of generated cols is supported, but not VIRTUAL cols. */ + 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 26796d4f9e..21f9ee7b84 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -784,7 +784,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) @@ -1106,6 +1106,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 c47eaf5523..361d9f3a0f 100644 --- a/src/test/subscription/t/011_generated.pl +++ b/src/test/subscription/t/011_generated.pl @@ -30,7 +30,7 @@ $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)" ); @@ -39,7 +39,7 @@ $node_subscriber->safe_psql('postgres', "CREATE TABLE tab2 (a int, b int)" ); -# publisher-side tab3 has generated col 'b' but subscriber-side tab2 has DIFFERENT COMPUTATION generated col 'b'. +# publisher-side tab3 has stored generated col 'b' but subscriber-side tab2 has DIFFERENT COMPUTATION generated col 'b'. $node_publisher->safe_psql('postgres', "CREATE TABLE tab3 (a int, b int GENERATED ALWAYS AS (a + 10) STORED)" ); @@ -48,7 +48,7 @@ $node_subscriber->safe_psql('postgres', "CREATE TABLE tab3 (a int, b int GENERATED ALWAYS AS (a + 20) STORED)" ); -# tab4: publisher-side generated col 'b' and 'c' --> subscriber-side non-generated col 'b', and generated-col 'c' +# tab4: publisher-side stored generated col 'b' and 'c' --> subscriber-side non-generated col 'b', and stored generated-col 'c' $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)" ); @@ -57,19 +57,21 @@ $node_subscriber->safe_psql('postgres', "CREATE TABLE tab4 (a int, b int, c int GENERATED ALWAYS AS (a * 22) STORED)" ); -# tab5: publisher-side non-generated col 'b' --> subscriber-side generated col 'b' +# tab5: publisher-side non-generated col 'b' --> subscriber-side 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)"); -# tab6: publisher-side generated col 'b' and 'c' --> subscriber-side non-generated col 'b', and generated-col 'c' +# tab6: publisher-side stored generated col 'b' and 'c' --> subscriber-side non-generated col 'b', and stored generated-col 'c' # columns on subscriber in different order $node_publisher->safe_psql('postgres', "CREATE TABLE tab6 (a int, b int GENERATED ALWAYS AS (a * 2) STORED, c int GENERATED ALWAYS AS (a * 2) STORED)"); $node_subscriber->safe_psql('postgres', "CREATE TABLE tab6 (c int GENERATED ALWAYS AS (a * 22) STORED, b int, a int)"); +# TODO: Add tests related to replication of VIRTUAL GNERATED COLUMNS + # data for initial sync $node_publisher->safe_psql('postgres', @@ -167,8 +169,8 @@ $node_publisher->safe_psql('postgres', "INSERT INTO tab4 VALUES (4), (5)"); $node_publisher->wait_for_catchup('sub4'); -# gen-col 'b' in publisher replicating to NOT gen-col 'b' on subscriber -# gen-col 'c' in publisher not replicating to gen-col 'c' on subscriber +# stored gen-col 'b' in publisher replicating to NOT gen-col 'b' on subscriber +# stored gen-col 'c' in publisher not replicating to stored gen-col 'c' on subscriber $result = $node_subscriber->safe_psql('postgres', "SELECT a, b, c FROM tab4 ORDER BY a"); @@ -182,7 +184,7 @@ $node_publisher->safe_psql('postgres', "INSERT INTO tab6 VALUES (4), (5)"); $node_publisher->wait_for_catchup('sub6'); -# gen-col 'b' and 'c' in publisher replicating to NOT gen-col 'b' and gen-col 'c' on subscriber +# stored gen-col 'b' and 'c' in publisher replicating to NOT gen-col 'b' and gen-col 'c' on subscriber # order of column is different on subscriber $result = $node_subscriber->safe_psql('postgres', "SELECT a, b, c FROM tab6 ORDER BY a"); @@ -192,7 +194,7 @@ is( $result, qq(1|2|22 4|8|88 5|10|110), 'replicate generated column with initial sync different column order'); -# NOT gen-col 'b' in publisher not replicating to gen-col 'b' on subscriber +# NOT gen-col 'b' in publisher not replicating to stored gen-col 'b' on subscriber my $offset = -s $node_subscriber->logfile; # sub5 will cause table sync worker to restart repetitively -- 2.41.0.windows.3