pgsql-hackers
❮
Include schema-qualified names in publication error messages.
- Jump to comment-1Dilip Kumar<dilipbalaut@gmail.com>Apr 28, 2026, 11:04 AM UTCPreviously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.
This patch updates these error messages to use schema-qualified names,
improving the clarity of error reporting for CREATE PUBLICATION and
This has been discussed on another thread [1]ALTER PUBLICATION commands.
[1] https://www.postgresql.org/message-id/CAFiTN-u3Si2XJM9PW0xVsOSoVfTGJZZq-TirZb3eON4rqG1EFw%40mail.gmail.com
--
Regards,
Dilip Kumar
Google- Jump to comment-1shveta malik<shveta.malik@gmail.com>Apr 28, 2026, 11:39 AM UTCOn Tue, Apr 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
+1
Previously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.This patch updates these error messages to use schema-qualified names,
The patch works well.
improving the clarity of error reporting for CREATE PUBLICATION and
ALTER PUBLICATION commands.
This has been discussed on another thread [1]
I think we can pull out
'getnamespacenameortemp(RelationGetNamespace(targetrel))' and
'RelationGetRelationName(targetrel)' into local variables to reduce
repetition and make the error paths a bit cleaner.
const char *nspname =
getnamespacenameortemp(RelationGetNamespace(targetrel));
const char *relname = RelationGetRelationName(targetrel);
thanks
Shveta- Jump to comment-1Peter Smith<smithpb2250@gmail.com>Apr 29, 2026, 3:58 AM UTCOn Tue, Apr 28, 2026 at 9:39 PM shveta malik <shveta.malik@gmail.com> wrote:
On Tue, Apr 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
Previously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.
+1This patch updates these error messages to use schema-qualified names,
improving the clarity of error reporting for CREATE PUBLICATION and
ALTER PUBLICATION commands.
This has been discussed on another thread [1]
How about having a dedicated function to return the fully qualified
The patch works well.
I think we can pull out
'getnamespacenameortemp(RelationGetNamespace(targetrel))' and
'RelationGetRelationName(targetrel)' into local variables to reduce
repetition and make the error paths a bit cleaner.
const char *nspname =
getnamespacenameortemp(RelationGetNamespace(targetrel));
const char *relname = RelationGetRelationName(targetrel);
relation name you want, which can then substitute the single %s.
e.g.
errmsg(errormsg, getqualifiedrelname(targetrel)), ...
======
Kind Regards,
Peter Smith.
Fujitsu Australia- Jump to comment-1Dilip Kumar<dilipbalaut@gmail.com>Apr 29, 2026, 10:39 AM UTCOn Wed, Apr 29, 2026 at 9:28 AM Peter Smith <smithpb2250@gmail.com> wrote:
On Tue, Apr 28, 2026 at 9:39 PM shveta malik <shveta.malik@gmail.com> wrote:
On Tue, Apr 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
Previously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.
+1This patch updates these error messages to use schema-qualified names,
improving the clarity of error reporting for CREATE PUBLICATION and
ALTER PUBLICATION commands.
This has been discussed on another thread [1]
The patch works well.
I think we can pull out
'getnamespacenameortemp(RelationGetNamespace(targetrel))' and
'RelationGetRelationName(targetrel)' into local variables to reduce
repetition and make the error paths a bit cleaner.
const char *nspname =
getnamespacenameortemp(RelationGetNamespace(targetrel));
const char *relname = RelationGetRelationName(targetrel);
Yeah that makes sense. I will change this.
How about having a dedicated function to return the fully qualified
relation name you want, which can then substitute the single %s.
e.g.
errmsg(errormsg, getqualifiedrelname(targetrel)), ...
--
Regards,
Dilip Kumar
Google- Jump to comment-1Dilip Kumar<dilipbalaut@gmail.com>Apr 29, 2026, 11:11 AM UTCOn Wed, Apr 29, 2026 at 4:08 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 9:28 AM Peter Smith <smithpb2250@gmail.com> wrote:
On Tue, Apr 28, 2026 at 9:39 PM shveta malik <shveta.malik@gmail.com> wrote:
On Tue, Apr 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
Previously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.
+1This patch updates these error messages to use schema-qualified names,
improving the clarity of error reporting for CREATE PUBLICATION and
ALTER PUBLICATION commands.
This has been discussed on another thread [1]
The patch works well.
I think we can pull out
'getnamespacenameortemp(RelationGetNamespace(targetrel))' and
'RelationGetRelationName(targetrel)' into local variables to reduce
repetition and make the error paths a bit cleaner.
const char *nspname =
getnamespacenameortemp(RelationGetNamespace(targetrel));
const char *relname = RelationGetRelationName(targetrel);
How about having a dedicated function to return the fully qualified
relation name you want, which can then substitute the single %s.
e.g.
errmsg(errormsg, getqualifiedrelname(targetrel)), ...
--
Yeah that makes sense. I will change this.
Regards,
Dilip Kumar
Google- Jump to comment-1shveta malik<shveta.malik@gmail.com>Apr 29, 2026, 11:32 AM UTCOn Wed, Apr 29, 2026 at 4:41 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:08 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 9:28 AM Peter Smith <smithpb2250@gmail.com> wrote:
On Tue, Apr 28, 2026 at 9:39 PM shveta malik <shveta.malik@gmail.com> wrote:
On Tue, Apr 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
Previously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.
+1This patch updates these error messages to use schema-qualified names,
improving the clarity of error reporting for CREATE PUBLICATION and
ALTER PUBLICATION commands.
This has been discussed on another thread [1]
The patch works well.
I think we can pull out
'getnamespacenameortemp(RelationGetNamespace(targetrel))' and
'RelationGetRelationName(targetrel)' into local variables to reduce
repetition and make the error paths a bit cleaner.
const char *nspname =
getnamespacenameortemp(RelationGetNamespace(targetrel));
const char *relname = RelationGetRelationName(targetrel);
How about having a dedicated function to return the fully qualified
relation name you want, which can then substitute the single %s.
e.g.
errmsg(errormsg, getqualifiedrelname(targetrel)), ...
Yeah that makes sense. I will change this.One trivial thing:
+/*
+ * Get a palloc'd string containing the schema-qualified name of the relation.
+ */
Extra space here: 'name of'
There is a similar function, generatequalifiedrelation_name(),
though I guess we can’t directly reuse it here. It takes a relid;
while we could extract the OID from the Relation and call it, that
seems a bit indirect when we already have the Relation in hand. In
that case, a local helper here seems reasonable. Right?
thanks
Shveta- Jump to comment-1Dilip Kumar<dilipbalaut@gmail.com>Apr 29, 2026, 12:32 PM UTCOn Wed, Apr 29, 2026 at 5:02 PM shveta malik <shveta.malik@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:41 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:08 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 9:28 AM Peter Smith <smithpb2250@gmail.com> wrote:
On Tue, Apr 28, 2026 at 9:39 PM shveta malik <shveta.malik@gmail.com> wrote:
On Tue, Apr 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
Previously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.
+1This patch updates these error messages to use schema-qualified names,
improving the clarity of error reporting for CREATE PUBLICATION and
ALTER PUBLICATION commands.
This has been discussed on another thread [1]
The patch works well.
I think we can pull out
'getnamespacenameortemp(RelationGetNamespace(targetrel))' and
'RelationGetRelationName(targetrel)' into local variables to reduce
repetition and make the error paths a bit cleaner.
const char *nspname =
getnamespacenameortemp(RelationGetNamespace(targetrel));
const char *relname = RelationGetRelationName(targetrel);
How about having a dedicated function to return the fully qualified
relation name you want, which can then substitute the single %s.
e.g.
errmsg(errormsg, getqualifiedrelname(targetrel)), ...
Yeah that makes sense. I will change this.
Oops, fixed now.
One trivial thing:
+/*
+ * Get a palloc'd string containing the schema-qualified name of the relation.
+ */
Extra space here: 'name of'There is a similar function, generatequalifiedrelation_name(),
Yeah, we already have a relation descriptor, so it's better to use that.
though I guess we can’t directly reuse it here. It takes a relid;
while we could extract the OID from the Relation and call it, that
seems a bit indirect when we already have the Relation in hand. In
that case, a local helper here seems reasonable. Right?
--
Regards,
Dilip Kumar
Google- Jump to comment-1shveta malik<shveta.malik@gmail.com>Apr 30, 2026, 4:17 AM UTCOn Wed, Apr 29, 2026 at 6:01 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 5:02 PM shveta malik <shveta.malik@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:41 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:08 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 9:28 AM Peter Smith <smithpb2250@gmail.com> wrote:
On Tue, Apr 28, 2026 at 9:39 PM shveta malik <shveta.malik@gmail.com> wrote:
On Tue, Apr 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
Previously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.
+1This patch updates these error messages to use schema-qualified names,
improving the clarity of error reporting for CREATE PUBLICATION and
ALTER PUBLICATION commands.
This has been discussed on another thread [1]
The patch works well.
I think we can pull out
'getnamespacenameortemp(RelationGetNamespace(targetrel))' and
'RelationGetRelationName(targetrel)' into local variables to reduce
repetition and make the error paths a bit cleaner.
const char *nspname =
getnamespacenameortemp(RelationGetNamespace(targetrel));
const char *relname = RelationGetRelationName(targetrel);
How about having a dedicated function to return the fully qualified
relation name you want, which can then substitute the single %s.
e.g.
errmsg(errormsg, getqualifiedrelname(targetrel)), ...
Yeah that makes sense. I will change this.
One trivial thing:
+/*
+ * Get a palloc'd string containing the schema-qualified name of the relation.
+ */
Extra space here: 'name of'
Oops, fixed now.There is a similar function, generatequalifiedrelation_name(),
though I guess we can’t directly reuse it here. It takes a relid;
while we could extract the OID from the Relation and call it, that
seems a bit indirect when we already have the Relation in hand. In
that case, a local helper here seems reasonable. Right?
+static char *getqualifiedrelname(Relation rel);
Yeah, we already have a relation descriptor, so it's better to use that.
This declaration seems unnecessary. We typically avoid adding one
unless it’s required due to a later definition being used earlier.
Other than that, the patch looks good.
thanks
Shveta- Jump to comment-1Dilip Kumar<dilipbalaut@gmail.com>Apr 30, 2026, 4:34 AM UTCOn Thu, Apr 30, 2026 at 9:47 AM shveta malik <shveta.malik@gmail.com> wrote:
On Wed, Apr 29, 2026 at 6:01 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 5:02 PM shveta malik <shveta.malik@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:41 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:08 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 9:28 AM Peter Smith <smithpb2250@gmail.com> wrote:
On Tue, Apr 28, 2026 at 9:39 PM shveta malik <shveta.malik@gmail.com> wrote:
On Tue, Apr 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
Previously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.
+1This patch updates these error messages to use schema-qualified names,
improving the clarity of error reporting for CREATE PUBLICATION and
ALTER PUBLICATION commands.
This has been discussed on another thread [1]
The patch works well.
I think we can pull out
'getnamespacenameortemp(RelationGetNamespace(targetrel))' and
'RelationGetRelationName(targetrel)' into local variables to reduce
repetition and make the error paths a bit cleaner.
const char *nspname =
getnamespacenameortemp(RelationGetNamespace(targetrel));
const char *relname = RelationGetRelationName(targetrel);
How about having a dedicated function to return the fully qualified
relation name you want, which can then substitute the single %s.
e.g.
errmsg(errormsg, getqualifiedrelname(targetrel)), ...
Yeah that makes sense. I will change this.
One trivial thing:
+/*
+ * Get a palloc'd string containing the schema-qualified name of the relation.
+ */
Extra space here: 'name of'
Oops, fixed now.There is a similar function, generatequalifiedrelation_name(),
though I guess we can’t directly reuse it here. It takes a relid;
while we could extract the OID from the Relation and call it, that
seems a bit indirect when we already have the Relation in hand. In
that case, a local helper here seems reasonable. Right?
Yeah, we already have a relation descriptor, so it's better to use that.
I generally prefer to add the static declaration irrespective of the
+static char *getqualifiedrelname(Relation rel);
This declaration seems unnecessary. We typically avoid adding one
unless it’s required due to a later definition being used earlier.
order. I understand your point, but that isn't uniformly followed,
though I prefer to add it.Other than that, the patch looks good.
Thanks.
--
Regards,
Dilip Kumar
Google
- Jump to comment-1vignesh C<vignesh21@gmail.com>Apr 30, 2026, 7:32 AM UTCOn Wed, 29 Apr 2026 at 18:02, Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 5:02 PM shveta malik <shveta.malik@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:41 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:08 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 9:28 AM Peter Smith <smithpb2250@gmail.com> wrote:
On Tue, Apr 28, 2026 at 9:39 PM shveta malik <shveta.malik@gmail.com> wrote:
On Tue, Apr 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
Previously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.
+1This patch updates these error messages to use schema-qualified names,
improving the clarity of error reporting for CREATE PUBLICATION and
ALTER PUBLICATION commands.
This has been discussed on another thread [1]
The patch works well.
I think we can pull out
'getnamespacenameortemp(RelationGetNamespace(targetrel))' and
'RelationGetRelationName(targetrel)' into local variables to reduce
repetition and make the error paths a bit cleaner.
const char *nspname =
getnamespacenameortemp(RelationGetNamespace(targetrel));
const char *relname = RelationGetRelationName(targetrel);
How about having a dedicated function to return the fully qualified
relation name you want, which can then substitute the single %s.
e.g.
errmsg(errormsg, getqualifiedrelname(targetrel)), ...
Yeah that makes sense. I will change this.
One trivial thing:
+/*
+ * Get a palloc'd string containing the schema-qualified name of the relation.
+ */
Extra space here: 'name of'
Oops, fixed now.There is a similar function, generatequalifiedrelation_name(),
though I guess we can’t directly reuse it here. It takes a relid;
while we could extract the OID from the Relation and call it, that
seems a bit indirect when we already have the Relation in hand. In
that case, a local helper here seems reasonable. Right?
We can remove the variables relname and result here by changing it to
Yeah, we already have a relation descriptor, so it's better to use that.
something like:
static char *
getqualifiedrelname(Relation rel)
{
}char *nspname; nspname = get_namespace_name_or_temp(RelationGetNamespace(rel)); if (!nspname) elog(ERROR, "cache lookup failed for namespace %u", RelationGetNamespace(rel)); return quote_qualified_identifier(nspname, RelationGetRelationName(rel));
Regards,
Vignesh- Jump to comment-1Dilip Kumar<dilipbalaut@gmail.com>Apr 30, 2026, 7:37 AM UTCOn Thu, Apr 30, 2026 at 1:02 PM vignesh C <vignesh21@gmail.com> wrote:
On Wed, 29 Apr 2026 at 18:02, Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 5:02 PM shveta malik <shveta.malik@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:41 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 4:08 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Apr 29, 2026 at 9:28 AM Peter Smith <smithpb2250@gmail.com> wrote:
On Tue, Apr 28, 2026 at 9:39 PM shveta malik <shveta.malik@gmail.com> wrote:
On Tue, Apr 28, 2026 at 4:34 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
Previously, error messages in checkpublicationadd_relation() only
reported the relation name when a table could not be added to a
publication or included in an EXCEPT clause. This could be ambiguous
in databases where the same relation name exists in multiple schemas.
+1This patch updates these error messages to use schema-qualified names,
improving the clarity of error reporting for CREATE PUBLICATION and
ALTER PUBLICATION commands.
This has been discussed on another thread [1]
The patch works well.
I think we can pull out
'getnamespacenameortemp(RelationGetNamespace(targetrel))' and
'RelationGetRelationName(targetrel)' into local variables to reduce
repetition and make the error paths a bit cleaner.
const char *nspname =
getnamespacenameortemp(RelationGetNamespace(targetrel));
const char *relname = RelationGetRelationName(targetrel);
How about having a dedicated function to return the fully qualified
relation name you want, which can then substitute the single %s.
e.g.
errmsg(errormsg, getqualifiedrelname(targetrel)), ...
Yeah that makes sense. I will change this.
One trivial thing:
+/*
+ * Get a palloc'd string containing the schema-qualified name of the relation.
+ */
Extra space here: 'name of'
Oops, fixed now.There is a similar function, generatequalifiedrelation_name(),
though I guess we can’t directly reuse it here. It takes a relid;
while we could extract the OID from the Relation and call it, that
seems a bit indirect when we already have the Relation in hand. In
that case, a local helper here seems reasonable. Right?
Yeah, we already have a relation descriptor, so it's better to use that.
Yeah we may, but I feel what we have now looks more readable.
We can remove the variables relname and result here by changing it to
something like:
static char *
getqualifiedrelname(Relation rel)
{
char *nspname;
nspname = getnamespacenameortemp(RelationGetNamespace(rel));
if (!nspname)
elog(ERROR, "cache lookup failed for namespace %u",
RelationGetNamespace(rel));
return quotequalifiedidentifier(nspname,
RelationGetRelationName(rel));
}
--
Regards,
Dilip Kumar
Google- Jump to comment-1Euler Taveira<euler@eulerto.com>Apr 30, 2026, 1:45 PM UTCOn Thu, Apr 30, 2026, at 4:37 AM, Dilip Kumar wrote:
My suggestion is that this function should be available in a central place.
Yeah we may, but I feel what we have now looks more readable.
That's not the only place that could use qualified schema and relation. If you
search for getnamespacenameortemp you will notice that this code path is
repeated in other parts of the code too (see ruleutils.c). It would be good if
we can have a common path for it. Maybe the signature has to be
getqualifiedrelname(Oid) to accommodate other cases.
--
Euler Taveira
EDB https://www.enterprisedb.com/- Jump to comment-1Dilip Kumar<dilipbalaut@gmail.com>Apr 30, 2026, 1:53 PM UTCOn Thu, 30 Apr 2026 at 7:14 PM, Euler Taveira <euler@eulerto.com> wrote:
On Thu, Apr 30, 2026, at 4:37 AM, Dilip Kumar wrote:
Yeah we may, but I feel what we have now looks more readable.
IMHO it’s not a good idea to use Oid when you already have reldesc.
My suggestion is that this function should be available in a central place.
That's not the only place that could use qualified schema and relation. If
you
search for getnamespacenameortemp you will notice that this code path
is
repeated in other parts of the code too (see ruleutils.c). It would be
good if
we can have a common path for it. Maybe the signature has to be
getqualifiedrelname(Oid) to accommodate
—
Dilip