Re: Include schema-qualified names in publication error messages.

On Thu, May 7, 2026 at 4:00 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
...
> > Okay, then we can split the patch into two, the first patch to make
> > the required changes only for EXCEPT, and the second one for the
> > remaining pre-existing messages. We can push the first patch in HEAD
> > and wait for some more opinions on the second one.
>

Hi Vignesh.

The patch had previously been split for EXCEPT and non-EXCEPT changes.

The 0001 patch was already pushed a while ago for PG19. I think now
that 0002 patch can be revisited for PG20.

======

IMO, the logical replication messages should consistently always give
fully qualified relation names in the error messages. The relation
named in the message can sometimes be ambiguous when not schema
qualified.

I saw some previous comment from Euler [1] saying we should refrain
from changing existing messages, but IMO here we are not rewording
message text for the sake of it; I think rather it is fixing the
values substituted to the *existing* messages to improve the clarity,
and at the same time making all the logical replication logs more
consistent. But, perhaps I misunderstood Euler's comment: if it was -1
referring only to backpatching then I agree.

Anyway, I looked again at the old v5-0002 patch. I found it is only
addressing the issue schema-qualification for
check_publication_add_relation. Actually, I think that is just a very
small part of something far bigger. e.g. There are many more places in
logical replication related code where fully qualified names could be
used.

Also, sometimes the log message is already schema-qualified, but is
not yet using the new "common" function designed for that purpose.

Below are dozens of examples that I found by searching the pub/sub
code for quoted relations/sequences/tables. Maybe you can find even
more.

======
Relations:

------
src/backend/commands/publicationcmds.c
TransformPubWhereClauses: errmsg("cannot use publication WHERE clause
for relation \"%s\"",
CheckPubRelationColumnList: errmsg("cannot use column list for
relation \"%s.%s\" in publication \"%s\"",
CheckPubRelationColumnList: errmsg("cannot use column list for
relation \"%s.%s\" in publication \"%s\"",
PublicationDropTables: errmsg("relation \"%s\" is not part of the publication",

------
src/backend/catalog/objectaddress.c
get_object_address_attribute: errmsg("column \"%s\" of relation \"%s\"
does not exist",
get_object_address_attrdef: errmsg("default value for column \"%s\" of
relation \"%s\" does not exist",
get_object_address_publication_rel: errmsg("publication relation
\"%s\" in publication \"%s\" does not exist",

------
src/backend/catalog/pg_publication.c
check_publication_add_relation: errormsg = gettext_noop("cannot add
relation \"%s\" to publication");
publication_add_relation: errmsg("relation \"%s\" is already member of
publication \"%s\"",
pub_collist_validate: errmsg("column \"%s\" of relation \"%s\" does not exist",

------
src/backend/catalog/pg_subscription.c
RemoveSubscriptionRel: errdetail("Table synchronization for relation
\"%s\" is in progress and is in state \"%c\".",

======
Sequences:

------
src/backend/commands/subscriptioncmds.c
AlterSubscription_refresh: errmsg_internal("sequence \"%s.%s\" removed
from subscription \"%s\"",
AlterSubscription_refresh_seq: errmsg_internal("sequence \"%s.%s\" of
subscription \"%s\" set to INIT state",

------
src/backend/replication/logical/sequencesync.c
copy_sequences: "logical replication synchronization for subscription
\"%s\", sequence \"%s.%s\" has finished",

======
Tables:

------
src/backend/commands/tablecmds.c
ATPrepChangePersistence: errmsg("cannot change table \"%s\" to
unlogged because it is part of a publication",
ATExecAttachPartition: errmsg_plural("cannot attach table \"%s\" as
partition because it is referenced in publication %s EXCEPT clause",
ATExecAttachPartition: cannot attach table \"%s\" as partition
because it is referenced in publications %s EXCEPT clause",

------
src/backend/commands/publicationcmds.c
AlterPublicationOptions: errdetail("The publication contains a WHERE
clause for partitioned table \"%s\", which is not allowed when \"%s\"
is false.",
AlterPublicationOptions: errdetail("The publication contains a column
list for partitioned table \"%s\", which is not allowed when \"%s\" is
false.",
OpenTableList: errmsg("conflicting or redundant WHERE clauses for table \"%s\"",
OpenTableList: errmsg("conflicting or redundant column lists for table \"%s\"",
OpenTableList: errmsg("conflicting or redundant WHERE clauses for table \"%s\"",
OpenTableList: errmsg("conflicting or redundant column lists for table \"%s\"",

------
src/backend/executor/execReplication.c
CheckCmdReplicaIdentity: errmsg("cannot update table \"%s\""
CheckCmdReplicaIdentity: errmsg("cannot update table \"%s\"",
CheckCmdReplicaIdentity: errmsg("cannot update table \"%s\"",
CheckCmdReplicaIdentity: errmsg("cannot delete from table \"%s\"",
CheckCmdReplicaIdentity: errmsg("cannot delete from table \"%s\"",
CheckCmdReplicaIdentity: errmsg("cannot delete from table \"%s\"",
- errmsg("cannot update table \"%s\" because it does not have a
replica identity and publishes updates",
- errmsg("cannot delete from table \"%s\" because it does not have a
replica identity and publishes deletes",

------
src/backend/replication/logical/syncutils.c
FinishSyncWorker: errmsg("logical replication table synchronization
worker for subscription \"%s\", table \"%s\" has finished",

------
src/backend/replication/logical/worker.c
InitializeLogRepWorker: errmsg("logical replication table
synchronization worker for subscription \"%s\", table \"%s\" has
started",

//////

Regarding the code of v5-0002 patch, my comments are below:

1.
Wasn't the newly pushed function called
'RelationGetQualifiedRelationName'. Maybe the name was changed after
some time after your v5* patches?

~~~

2.
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg(errormsg, relname),
+ errmsg(errormsg, get_relation_qualified_name(targetrel)),
errdetail("This operation is not supported for individual partitions.")));

That "get_relation_qualified_name(targetrel)" is repeated many times.
IMO the code would be neater to just assign the name up-front to a
variable.

======
[1] https://www.postgresql.org/message-id/d0979f9c-10a3-4983-8a41-7014135d02f9%40app.fastmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia