From 291cb87e8e3e1ac84c2dca3703c764be26331148 Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 26 Nov 2024 18:33:04 +1100 Subject: [PATCH v7] 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 --- src/backend/replication/logical/relation.c | 91 +++++++++++++++------- src/test/subscription/t/011_generated.pl | 42 ++++++++++ 2 files changed, 103 insertions(+), 30 deletions(-) diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index f5a0ef2bd9..20af65d5a0 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 + * random. + */ +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 211b54c316..2f55581a11 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 for expected error when attempting to replicate to a +# generated subscriber column. Test the following combination +# - 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(); -- 2.28.0.windows.1