Thread

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

    Peter Smith <smithpb2250@gmail.com> — 2026-05-27T06:20:05Z

    On Thu, May 7, 2026 at 4:00 PM vignesh C <vignesh21@gmail.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