Schema-Qualified Names in Publication Error Messages

Core Problem

The function check_publication_add_relation() in src/backend/catalog/pg_publication.c emits several ERROR messages when a relation cannot be added to a publication (e.g., for system catalogs, unlogged tables, temporary tables, partitioned indexes) or cannot appear in an EXCEPT clause. Historically, these messages embedded only the unqualified relation name via RelationGetRelationName().

In databases where the same table name exists in multiple schemas (a very common situation — think app.orders vs archive.orders), an error like:

ERROR: "orders" is a system table

is ambiguous: the user cannot tell which relation was rejected. This matters especially for logical replication management where publications often reference many relations across schemas, and for the newer EXCEPT clause (introduced by commit fd366065e06a for PG18) which broadens the surface area where such errors fire.

Architecturally this is a small diagnostic/UX fix, not a correctness fix — but consistency of error reporting matters because (a) it reduces user confusion for logical replication setup, and (b) log-analysis tools and user expectations key off message shape.

Proposed Solution and Its Evolution

The initial patch simply inlined get_namespace_name_or_temp(RelationGetNamespace(targetrel)) together with RelationGetRelationName(targetrel) into each errmsg() call site. Review quickly pushed the design through several iterations:

1. Local variables (Shveta): pull the nspname/relname out to avoid repetition — a mechanical cleanup.
2. Dedicated helper (Peter Smith): encapsulate the qualification into a single function returning a palloc'd schema-qualified identifier, so each call site remains a single %s. This became get_qualified_relname(Relation rel) in pg_publication.c.
3. Centralization debate (Euler Taveira): the logic duplicates what ruleutils.c:generate_qualified_relation_name() already does, and other sites use the get_namespace_name_or_temp pattern. Euler argued for a shared helper keyed on Oid so all call sites could converge.
4. Relation vs Oid (Dilip): when the caller already holds a Relation descriptor, taking an Oid forces an unnecessary syscache lookup. Shveta surveyed call sites and confirmed no other get_namespace_name_or_temp user already has a Relation in hand — so the Relation-taking helper is genuinely unique to this path.
5. Final factoring (Amit Kapila): for HEAD, extract the common core — taking (nspid, relname) — into lsyscache.c, and let both generate_qualified_relation_name() (Oid-based) and the new get_qualified_relname() (Relation-based) call into it. This gives the best of both worlds: shared lookup/quoting logic, but each caller passes what it naturally holds.

The quote_qualified_identifier(nspname, relname) call at the core ensures proper double-quoting of identifiers that require it — critical because error messages that later appear in scripts or documentation should be copy-pasteable.

Minor style debates (whether to keep a forward static declaration when the definition precedes all uses; whether to collapse locals in get_qualified_relname) were resolved in favor of readability/consistency with author preference — Dilip (the author) holds the pen here.

Backpatching Decision

The more substantive design question was scope of application:

• Amit Kapila initially raised backpatching, noting the behavior "is not correct" even without a bug report.
• Euler Taveira pushed back strongly: the messages have been this way "since the beginning," and silently changing WARNING/ERROR/FATAL/PANIC message text in stable branches breaks log-analysis tooling that matches on message shape. This is a well-established PostgreSQL project principle — stable branches preserve message text precisely because operators grep and alert on it.
• Compromise: split the patch. Commit b49086a7 mentioned nominally — the EXCEPT-clause messages are new in PG18 (via fd366065e06a), so qualifying those is effectively fixing a message that hasn't calcified yet. The older messages (system table, temp table, unlogged, partitioned index rejections) will be changed only on HEAD (v20 target, given v19 feature freeze timing discussion).

This two-patch split is the key architectural outcome: it distinguishes "fixing a freshly-introduced suboptimal message" from "changing long-standing message text," and applies different stability policies to each — deferring the latter to the RMT (Release Management Team).

Why This Matters

While the patch is small, it illustrates several recurring PostgreSQL review patterns:

• Helper placement: the tension between local single-use helpers and cross-module utilities, resolved by factoring the shared primitive (in lsyscache.c) while keeping thin wrappers for different input types.
• Oid vs Relation APIs: a recurring internals question — prefer the form that avoids re-doing syscache lookups the caller has already done.
• Message stability as a contract: error message text is part of PostgreSQL's de facto external API for operators. The decision process around backpatching reflects this contract explicitly.
• Feature-freeze timing: the discussion occurs post-PG19 feature freeze, forcing explicit classification of the fix (not a feature, arguably a bug, but not user-reported) and a decision on whether to slip it into v19 or target v20.