From 9ce9594405422bb07728125bc6ba0e2e10b61bb8 Mon Sep 17 00:00:00 2001 From: Shubham Khanna Date: Thu, 7 Nov 2024 15:54:19 +0530 Subject: [PATCH v4] Error-message-improvement Currently, if logical replication attempts to target a subscriber-side table column that is either missing or generated, it produces the following identical error message: ERROR: logical replication target relation \"%s.%s\" is missing replicated columns: %s While the error itself is valid, the message wording can be misleading for generated columns. This patch introduces a distinct error message specifically for the generated column scenario. --- src/backend/replication/logical/relation.c | 77 +++++++++++++++------- src/test/subscription/t/011_generated.pl | 76 +++++++++++++++++++++ 2 files changed, 129 insertions(+), 24 deletions(-) diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index f5a0ef2bd9..2c7be7d154 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -219,41 +219,53 @@ logicalrep_rel_att_by_name(LogicalRepRelation *remoterel, const char *attname) return -1; } -/* - * Report error with names of the missing local relation column(s), if any. +/* If ismissing is true, report the error message as 'Missing replicated + * columns.' Otherwise, report the error message as 'Cannot replicate to + * generated columns.' */ static void -logicalrep_report_missing_attrs(LogicalRepRelation *remoterel, - Bitmapset *missingatts) +logicalrep_report_missing_and_gen_attrs(LogicalRepRelation *remoterel, + Bitmapset *atts, + bool ismissing) { - if (!bms_is_empty(missingatts)) + if (!bms_is_empty(atts)) { - StringInfoData missingattsbuf; - int missingattcnt = 0; + StringInfoData attsbuf; + int attcnt = 0; int i; - initStringInfo(&missingattsbuf); + initStringInfo(&attsbuf); i = -1; - while ((i = bms_next_member(missingatts, i)) >= 0) + while ((i = bms_next_member(atts, i)) >= 0) { - missingattcnt++; - if (missingattcnt == 1) - appendStringInfo(&missingattsbuf, _("\"%s\""), + attcnt++; + if (attcnt == 1) + appendStringInfo(&attsbuf, _("\"%s\""), remoterel->attnames[i]); else - appendStringInfo(&missingattsbuf, _(", \"%s\""), + appendStringInfo(&attsbuf, _(", \"%s\""), remoterel->attnames[i]); } - ereport(ERROR, - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg_plural("logical replication target relation \"%s.%s\" is missing replicated column: %s", - "logical replication target relation \"%s.%s\" is missing replicated columns: %s", - missingattcnt, - remoterel->nspname, - remoterel->relname, - missingattsbuf.data))); + if (ismissing) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg_plural("logical replication target relation \"%s.%s\" is missing replicated column: %s", + "logical replication target relation \"%s.%s\" is missing replicated columns: %s", + attcnt, + remoterel->nspname, + remoterel->relname, + attsbuf.data))); + else + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg_plural("cannot replicate to target relation \"%s.%s\" generated column: %s", + "cannot replicate to target relation \"%s.%s\" generated columns: %s", + attcnt, + remoterel->nspname, + remoterel->relname, + attsbuf.data))); } } @@ -379,7 +391,9 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) TupleDesc desc; MemoryContext oldctx; int i; - Bitmapset *missingatts; + Bitmapset *missingatts; /* Bitmapset for missing attributes. */ + Bitmapset *generatedattrs = NULL; /* Bitmapset for generated + * attributes. */ /* Release the no-longer-useful attrmap, if any. */ if (entry->attrmap) @@ -421,7 +435,7 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) int attnum; Form_pg_attribute attr = TupleDescAttr(desc, i); - if (attr->attisdropped || attr->attgenerated) + if (attr->attisdropped) { entry->attrmap->attnums[i] = -1; continue; @@ -432,12 +446,27 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) entry->attrmap->attnums[i] = attnum; if (attnum >= 0) + { + /* + * Include it in generatedattrs if publishing to a generated + * column. + */ + if (attr->attgenerated) + generatedattrs = bms_add_member(generatedattrs, attnum); + missingatts = bms_del_member(missingatts, attnum); + } } - logicalrep_report_missing_attrs(remoterel, missingatts); + /* Report replicating to generated columns. */ + logicalrep_report_missing_and_gen_attrs(remoterel, generatedattrs, + false); + /* Report missing columns. */ + logicalrep_report_missing_and_gen_attrs(remoterel, missingatts, + true); /* be tidy */ + bms_free(generatedattrs); bms_free(missingatts); /* diff --git a/src/test/subscription/t/011_generated.pl b/src/test/subscription/t/011_generated.pl index 211b54c316..2a2df363c0 100644 --- a/src/test/subscription/t/011_generated.pl +++ b/src/test/subscription/t/011_generated.pl @@ -326,4 +326,80 @@ is( $result, qq(|2| $node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub1"); $node_publisher->safe_psql('postgres', "DROP PUBLICATION pub1"); +# ============================================================================= +# The following test cases exercise logical replication for the combinations +# where there is a generated column on one or both sides of pub/sub: +# - regular -> generated and generated -> generated +# - regular -> missing +# ============================================================================= + +# -------------------------------------------------- +# A "regular -> generated" and "generated -> generated" replication fails, +# reporting an error that the generated column on the subscriber side +# cannot be replicated. +# +# Test Case: regular -> generated and generated -> generated +# Publisher table has regular column 'c2' and generated column 'c3'. +# Subscriber table has generated columns 'c2' and 'c3'. +# -------------------------------------------------- + +# Create table and publication. Insert data into the table. +$node_publisher->safe_psql( + 'postgres', qq( + CREATE TABLE t1(c1 int, c2 int, c3 int GENERATED ALWAYS AS (c1 * 2) STORED); + CREATE PUBLICATION pub1 for table t1(c1, c2, c3); + INSERT INTO t1 VALUES (1); +)); + +# Create table and subscription. +$node_subscriber->safe_psql( + 'postgres', qq( + CREATE TABLE t1(c1 int, c2 int GENERATED ALWAYS AS (c1 + 2) STORED, c3 int GENERATED ALWAYS AS (c1 + 2) STORED); + CREATE SUBSCRIPTION sub1 CONNECTION '$publisher_connstr' PUBLICATION pub1; +)); + +# Verify that an error occurs. +my $offset = -s $node_subscriber->logfile; +$node_subscriber->wait_for_log( + qr/ERROR: ( [A-Z0-9]:)? cannot replicate to target relation "public.t1" generated columns: "c2", "c3"/, + $offset); + +# cleanup +$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub1"); +$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub1"); + +# -------------------------------------------------- +# A "regular -> missing" replication fails, reporting an error +# that the subscriber side is missing replicated columns. +# +# Testcase: regular -> missing +# Publisher table has regular columns 'c2' and 'c3'. +# Subscriber table is missing columns 'c2' and 'c3'. +# -------------------------------------------------- + +# Create table and publication. Insert data into the table. +$node_publisher->safe_psql( + 'postgres', qq( + CREATE TABLE t2(c1 int, c2 int, c3 int); + CREATE PUBLICATION pub1 for table t2(c1, c2, c3); + INSERT INTO t2 VALUES (1); +)); + +# Create table and subscription. +$node_subscriber->safe_psql( + 'postgres', qq( + CREATE TABLE t2(c1 int); + CREATE SUBSCRIPTION sub1 CONNECTION '$publisher_connstr' PUBLICATION pub1; +)); + +# Verify that an error occurs. +$offset = -s $node_subscriber->logfile; +$node_subscriber->wait_for_log( + qr/ERROR: ( [A-Z0-9]:)? logical replication target relation "public.t2" is missing replicated columns: "c2", "c3"/, + $offset); + +# cleanup +$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub1"); +$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub1"); + done_testing(); -- 2.41.0.windows.3