From: Amit Kapila Date: Wed, 27 Nov 2024 03:39:20 +0000 (+0530) Subject: Improve error message for replication of generated columns. X-Git-Tag: REL_18_BETA1~1412 X-Git-Url: http://git.postgresql.org/gitweb/?a=commitdiff_plain;h=8fcd80258bcf43dab93d877a5de0ce3f4d2bd471;p=postgresql.git Improve error message for replication of generated columns. Currently, logical replication produces a generic error message when targeting a subscriber-side table column that is either missing or generated. The error message can be misleading for generated columns. This patch introduces a specific error message to clarify the issue when generated columns are involved. Author: Shubham Khanna Reviewed-by: Peter Smith, Vignesh C, Amit Kapila Discussion: https://postgr.es/m/CAHv8RjJBvYtqU7OAofBizOmQOK2Q8h+w9v2_cQWxT_gO7er3Aw@mail.gmail.com --- diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index f5a0ef2bd9d..b355f32f03c 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -220,41 +220,63 @@ 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 missing or generated columns, report an error. + * Prioritize 'missing' errors if both occur though the prioritization is + * arbitrary. + */ +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("logical replication target relation \"%s.%s\" has incompatible generated column: %s", + "logical replication target relation \"%s.%s\" has incompatible generated columns: %s", + bms_num_members(generatedatts), + remoterel->nspname, + remoterel->relname, + logicalrep_get_attrs_str(remoterel, + generatedatts))); } /* @@ -380,6 +402,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 +444,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 +455,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 211b54c3162..66e6d8da5ac 100644 --- a/src/test/subscription/t/011_generated.pl +++ b/src/test/subscription/t/011_generated.pl @@ -326,4 +326,46 @@ is( $result, qq(|2| $node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub1"); $node_publisher->safe_psql('postgres', "DROP PUBLICATION pub1"); +# ============================================================================= +# The following test verifies the expected error when replicating to a +# generated subscriber column. Test the following combinations: +# - regular -> generated +# - generated -> generated +# ============================================================================= + +# -------------------------------------------------- +# 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]:)? logical replication target relation "public.t1" has incompatible generated columns: "c2", "c3"/, + $offset); + +# cleanup +$node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION sub1"); +$node_publisher->safe_psql('postgres', "DROP PUBLICATION pub1"); + done_testing();