From 4afbe9262889a5e37191d80abbde0301fca83c46 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 26 Nov 2024 18:33:04 +1100 Subject: [PATCH v6] 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. Note that if there are both kinds of errors present (missing columns and generated columns) the 'missing' error takes precedence. --- src/backend/replication/logical/relation.c | 92 +++++++++++++++------- src/test/subscription/t/011_generated.pl | 83 +++++++++++++++++++ 2 files changed, 145 insertions(+), 30 deletions(-) diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index f5a0ef2bd9..91d69a85a1 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -220,41 +220,64 @@ logicalrep_rel_att_by_name(LogicalRepRelation *remoterel, const char *attname) } /* - * Report error with names of the missing local relation column(s), if any. + * Returns a comma-separated string of attribute names based on the provided + * relation and bitmap indicating which attributes to include. */ -static void -logicalrep_report_missing_attrs(LogicalRepRelation *remoterel, - Bitmapset *missingatts) +static char * +logicalrep_get_attrs_str(LogicalRepRelation *remoterel, Bitmapset *atts) { - if (!bms_is_empty(missingatts)) + StringInfoData attsbuf; + int attcnt = 0; + int i = -1; + + Assert(!bms_is_empty(atts)); + + initStringInfo(&attsbuf); + + while ((i = bms_next_member(atts, i)) >= 0) { - StringInfoData missingattsbuf; - int missingattcnt = 0; - int i; + attcnt++; + if (attcnt > 1) + appendStringInfo(&attsbuf, _(", ")); - initStringInfo(&missingattsbuf); + appendStringInfo(&attsbuf, _("\"%s\""), remoterel->attnames[i]); + } - i = -1; - while ((i = bms_next_member(missingatts, i)) >= 0) - { - missingattcnt++; - if (missingattcnt == 1) - appendStringInfo(&missingattsbuf, _("\"%s\""), - remoterel->attnames[i]); - else - appendStringInfo(&missingattsbuf, _(", \"%s\""), - remoterel->attnames[i]); - } + return attsbuf.data; +} +/* + * If attempting to replicate to subscriber side missing columns or generated + * columns then report an error. + * + * (If both problems exist then the 'missing' error takes precedence). + */ +static void +logicalrep_report_missing_or_gen_attrs(LogicalRepRelation *remoterel, + Bitmapset *missingatts, + Bitmapset *generatedatts) +{ + if (!bms_is_empty(missingatts)) 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))); - } + 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", + bms_num_members(missingatts), + remoterel->nspname, + remoterel->relname, + logicalrep_get_attrs_str(remoterel, + missingatts))); + + if (!bms_is_empty(generatedatts)) + 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", + bms_num_members(generatedatts), + remoterel->nspname, + remoterel->relname, + logicalrep_get_attrs_str(remoterel, + generatedatts))); } /* @@ -380,6 +403,7 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) MemoryContext oldctx; int i; Bitmapset *missingatts; + Bitmapset *generatedattrs = NULL; /* Release the no-longer-useful attrmap, if any. */ if (entry->attrmap) @@ -421,7 +445,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 +456,20 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) entry->attrmap->attnums[i] = attnum; if (attnum >= 0) + { + /* Remember which subscriber columns are generated. */ + if (attr->attgenerated) + generatedattrs = bms_add_member(generatedattrs, attnum); + missingatts = bms_del_member(missingatts, attnum); + } } - logicalrep_report_missing_attrs(remoterel, missingatts); + logicalrep_report_missing_or_gen_attrs(remoterel, missingatts, + generatedattrs); /* 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..977e3b1fd4 100644 --- a/src/test/subscription/t/011_generated.pl +++ b/src/test/subscription/t/011_generated.pl @@ -326,4 +326,87 @@ is( $result, qq(|2| $node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub1"); $node_publisher->safe_psql('postgres', "DROP PUBLICATION pub1"); +# ============================================================================= +# The following tests for expected errors when attempting to replicate to a +# missing or a generated subscriber column. Test the following combinations +# - regular -> generated +# - generated -> generated +# - regular -> missing +# - generated -> missing +# - test if both errors exist, then the missing error is reported first. +# ============================================================================= + +# -------------------------------------------------- +# A "regular -> generated" or "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" or "generated -> missing" replication fails, +# reporting an error that the subscriber side is missing replicated columns. +# +# Testcase: regular -> missing and generated -> missing +# Publisher table has regular column 'c4' and generated column 'c5'. +# Subscriber table is missing columns 'c4' and 'c5'. +# +# Notice how the first 3 columns of t2 are identical to the previous test, +# so this table also has generated column errors, but they are not reported +# because the 'missing' column error takes precedence. +# -------------------------------------------------- + +# Create table and publication. Insert data into the table. +$node_publisher->safe_psql( + 'postgres', qq( + CREATE TABLE t2(c1 int, c2 int, c3 int GENERATED ALWAYS AS (c1 * 2) STORED, c4 int, c5 int GENERATED ALWAYS AS (c1 * 2) STORED); + CREATE PUBLICATION pub1 for table t2(c1, c2, c3, c4, c5); + INSERT INTO t2 VALUES (1); +)); + +# Create table and subscription. +$node_subscriber->safe_psql( + 'postgres', qq( + CREATE TABLE t2(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. +$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: "c4", "c5"/, + $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