Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Use schema-qualified names in EXCEPT clause error messages.

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

    shveta malik <shveta.malik@gmail.com> — 2026-05-04T05:21:07Z

    On Thu, Apr 30, 2026 at 7:23 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On 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.
>> >
>>
>> 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 get_namespace_name_or_temp 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
>> get_qualified_relname(Oid) to accommodate
>
>
> IMHO it’s not a good idea to use Oid when you already have reldesc.

+1.

I looked at other use cases of get_namespace_name_or_temp(), and there
doesn’t seem to be any case where we already have a Relation
descriptor. So this appears to be a unique scenario, and I feel adding
a new function here makes sense. If needed, ruleutils.c’s
generate_qualified_relation_name() could be moved to a common location
in a separate patch.

Thanks,
Shveta

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-05-05T04:57:47Z

    On Mon, May 4, 2026 at 10:51 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Thu, Apr 30, 2026 at 7:23 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On 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.
> >> >
> >>
> >> 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 get_namespace_name_or_temp 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
> >> get_qualified_relname(Oid) to accommodate
> >
> >
> > IMHO it’s not a good idea to use Oid when you already have reldesc.
>
> +1.
>
> I looked at other use cases of get_namespace_name_or_temp(), and there
> doesn’t seem to be any case where we already have a Relation
> descriptor. So this appears to be a unique scenario, and I feel adding
> a new function here makes sense. If needed, ruleutils.c’s
> generate_qualified_relation_name() could be moved to a common location
> in a separate patch.
>

I think for HEAD, we can move the common part of
generate_qualified_relation_name() and get_qualified_relname() to a
common function that takes relname as input. We can probably move it
to lsyscache.c.

Now, we also need to decide whether to backpatch the relevant change
to back-branches. It seems we didn't get the bug-report yet but
clearly what we do currently is not correct. So, we should ideally
backpatch it and in the back branches we don't need to expose it.
OTOH, as it is reported and is not a big issue, so we can keep this as
a HEAD only change as well. If we want to keep this as a HEAD only
change then shall we wait for PG20 branch to open or go for current
HEAD itself? What do you and or others think on this matter?

-- 
With Regards,
Amit Kapila.

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

    shveta malik <shveta.malik@gmail.com> — 2026-05-05T05:35:41Z

    On Tue, May 5, 2026 at 10:28 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, May 4, 2026 at 10:51 AM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Thu, Apr 30, 2026 at 7:23 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On 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.
> > >> >
> > >>
> > >> 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 get_namespace_name_or_temp 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
> > >> get_qualified_relname(Oid) to accommodate
> > >
> > >
> > > IMHO it’s not a good idea to use Oid when you already have reldesc.
> >
> > +1.
> >
> > I looked at other use cases of get_namespace_name_or_temp(), and there
> > doesn’t seem to be any case where we already have a Relation
> > descriptor. So this appears to be a unique scenario, and I feel adding
> > a new function here makes sense. If needed, ruleutils.c’s
> > generate_qualified_relation_name() could be moved to a common location
> > in a separate patch.
> >
>
> I think for HEAD, we can move the common part of
> generate_qualified_relation_name() and get_qualified_relname() to a
> common function that takes relname as input. We can probably move it
> to lsyscache.c.
>
> Now, we also need to decide whether to backpatch the relevant change
> to back-branches. It seems we didn't get the bug-report yet but
> clearly what we do currently is not correct. So, we should ideally
> backpatch it and in the back branches we don't need to expose it.
> OTOH, as it is reported and is not a big issue, so we can keep this as
> a HEAD only change as well.

+1, it is a rare error-case scenario and it has not been reported so
far despite of change being present for a long time. So I think the
HEAD-only change is good.

> If we want to keep this as a HEAD only
> change then shall we wait for PG20 branch to open or go for current
> HEAD itself? What do you and or others think on this matter?
>

I am fine with both. Again, a rare scenario that hasn't reported yet,
we can wait for the Pg20 branch to open.

thanks
Shveta

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

    Dilip Kumar <dilipbalaut@gmail.com> — 2026-05-05T10:32:27Z

    On Tue, May 5, 2026 at 10:27 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, May 4, 2026 at 10:51 AM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Thu, Apr 30, 2026 at 7:23 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On 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.
> > >> >
> > >>
> > >> 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 get_namespace_name_or_temp 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
> > >> get_qualified_relname(Oid) to accommodate
> > >
> > >
> > > IMHO it’s not a good idea to use Oid when you already have reldesc.
> >
> > +1.
> >
> > I looked at other use cases of get_namespace_name_or_temp(), and there
> > doesn’t seem to be any case where we already have a Relation
> > descriptor. So this appears to be a unique scenario, and I feel adding
> > a new function here makes sense. If needed, ruleutils.c’s
> > generate_qualified_relation_name() could be moved to a common location
> > in a separate patch.
> >
>
> I think for HEAD, we can move the common part of
> generate_qualified_relation_name() and get_qualified_relname() to a
> common function that takes relname as input. We can probably move it
> to lsyscache.c.

Done that, I think we need to pass nspid and relname.

> Now, we also need to decide whether to backpatch the relevant change
> to back-branches. It seems we didn't get the bug-report yet but
> clearly what we do currently is not correct. So, we should ideally
> backpatch it and in the back branches we don't need to expose it.
> OTOH, as it is reported and is not a big issue, so we can keep this as
> a HEAD only change as well. If we want to keep this as a HEAD only
> change then shall we wait for PG20 branch to open or go for current
> HEAD itself? What do you and or others think on this matter?

I think we should apply in PG19. Although back-patching isn't
critical, since we already have an opportunity to fix it in PG19, why
not push it early?

-- 
Regards,
Dilip Kumar
Google

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-05-05T10:42:08Z

    On Tue, May 5, 2026 at 4:02 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> > Now, we also need to decide whether to backpatch the relevant change
> > to back-branches. It seems we didn't get the bug-report yet but
> > clearly what we do currently is not correct. So, we should ideally
> > backpatch it and in the back branches we don't need to expose it.
> > OTOH, as it is reported and is not a big issue, so we can keep this as
> > a HEAD only change as well. If we want to keep this as a HEAD only
> > change then shall we wait for PG20 branch to open or go for current
> > HEAD itself? What do you and or others think on this matter?
>
> I think we should apply in PG19. Although back-patching isn't
> critical, since we already have an opportunity to fix it in PG19, why
> not push it early?
>

I also think we should push it for PG19 especially because the EXCEPT
feature increased the usage of relation names without schema-name in
error messages. However, as we are past feature freeze, I wanted to
know the opinion of others as well.

-- 
With Regards,
Amit Kapila.

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

    Euler Taveira <euler@eulerto.com> — 2026-05-05T12:24:48Z

    On Tue, May 5, 2026, at 7:42 AM, Amit Kapila wrote:
> On Tue, May 5, 2026 at 4:02 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>>
>> > Now, we also need to decide whether to backpatch the relevant change
>> > to back-branches. It seems we didn't get the bug-report yet but
>> > clearly what we do currently is not correct. So, we should ideally
>> > backpatch it and in the back branches we don't need to expose it.
>> > OTOH, as it is reported and is not a big issue, so we can keep this as
>> > a HEAD only change as well. If we want to keep this as a HEAD only
>> > change then shall we wait for PG20 branch to open or go for current
>> > HEAD itself? What do you and or others think on this matter?
>>
>> I think we should apply in PG19. Although back-patching isn't
>> critical, since we already have an opportunity to fix it in PG19, why
>> not push it early?
>>
>
> I also think we should push it for PG19 especially because the EXCEPT
> feature increased the usage of relation names without schema-name in
> error messages. However, as we are past feature freeze, I wanted to
> know the opinion of others as well.
>

-1 for backpatching. These messages (without schema qualification) has been
like this since the beginning. The function was not introduced by fd366065e06a
and the proposed patch are changing existing messages as well. It is a good
idea to keep visible messages (WARNING, ERROR, FATAL, PANIC) consistent so as
not to break log analysis tools.

I would say the target is v20. However, as Amit said, the change to the EXCEPT
clause message might be important, so I suggest changing it; I would leave the
other messages for the RMT to decide.

PS> since we don't have a REL_19_STABLE branch yet, backpatch refers to v18 or
earlier.


-- 
Euler Taveira
EDB   https://www.enterprisedb.com/

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-05-06T03:57:30Z

    On Tue, May 5, 2026 at 5:56 PM Euler Taveira <euler@eulerto.com> wrote:
>
> On Tue, May 5, 2026, at 7:42 AM, Amit Kapila wrote:
> > On Tue, May 5, 2026 at 4:02 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >>
> >> > Now, we also need to decide whether to backpatch the relevant change
> >> > to back-branches. It seems we didn't get the bug-report yet but
> >> > clearly what we do currently is not correct. So, we should ideally
> >> > backpatch it and in the back branches we don't need to expose it.
> >> > OTOH, as it is reported and is not a big issue, so we can keep this as
> >> > a HEAD only change as well. If we want to keep this as a HEAD only
> >> > change then shall we wait for PG20 branch to open or go for current
> >> > HEAD itself? What do you and or others think on this matter?
> >>
> >> I think we should apply in PG19. Although back-patching isn't
> >> critical, since we already have an opportunity to fix it in PG19, why
> >> not push it early?
> >>
> >
> > I also think we should push it for PG19 especially because the EXCEPT
> > feature increased the usage of relation names without schema-name in
> > error messages. However, as we are past feature freeze, I wanted to
> > know the opinion of others as well.
> >
>
> -1 for backpatching.
>

Agreed.

> These messages (without schema qualification) has been
> like this since the beginning. The function was not introduced by fd366065e06a
> and the proposed patch are changing existing messages as well. It is a good
> idea to keep visible messages (WARNING, ERROR, FATAL, PANIC) consistent so as
> not to break log analysis tools.
>
> I would say the target is v20. However, as Amit said, the change to the EXCEPT
> clause message might be important, so I suggest changing it; I would leave the
> other messages for the RMT to decide.
>

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.

-- 
With Regards,
Amit Kapila.

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

    vignesh C <vignesh21@gmail.com> — 2026-05-07T06:00:02Z

    On Wed, 6 May 2026 at 09:27, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, May 5, 2026 at 5:56 PM Euler Taveira <euler@eulerto.com> wrote:
> >
> > On Tue, May 5, 2026, at 7:42 AM, Amit Kapila wrote:
> > > On Tue, May 5, 2026 at 4:02 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >>
> > >> > Now, we also need to decide whether to backpatch the relevant change
> > >> > to back-branches. It seems we didn't get the bug-report yet but
> > >> > clearly what we do currently is not correct. So, we should ideally
> > >> > backpatch it and in the back branches we don't need to expose it.
> > >> > OTOH, as it is reported and is not a big issue, so we can keep this as
> > >> > a HEAD only change as well. If we want to keep this as a HEAD only
> > >> > change then shall we wait for PG20 branch to open or go for current
> > >> > HEAD itself? What do you and or others think on this matter?
> > >>
> > >> I think we should apply in PG19. Although back-patching isn't
> > >> critical, since we already have an opportunity to fix it in PG19, why
> > >> not push it early?
> > >>
> > >
> > > I also think we should push it for PG19 especially because the EXCEPT
> > > feature increased the usage of relation names without schema-name in
> > > error messages. However, as we are past feature freeze, I wanted to
> > > know the opinion of others as well.
> > >
> >
> > -1 for backpatching.
> >
>
> Agreed.
>
> > These messages (without schema qualification) has been
> > like this since the beginning. The function was not introduced by fd366065e06a
> > and the proposed patch are changing existing messages as well. It is a good
> > idea to keep visible messages (WARNING, ERROR, FATAL, PANIC) consistent so as
> > not to break log analysis tools.
> >
> > I would say the target is v20. However, as Amit said, the change to the EXCEPT
> > clause message might be important, so I suggest changing it; I would leave the
> > other messages for the RMT to decide.
> >
>
> 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.

The updated patch has the changes to split it.

Regards,
Vignesh

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

    shveta malik <shveta.malik@gmail.com> — 2026-05-07T09:33:09Z

    On Thu, May 7, 2026 at 11:30 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Wed, 6 May 2026 at 09:27, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Tue, May 5, 2026 at 5:56 PM Euler Taveira <euler@eulerto.com> wrote:
> > >
> > > On Tue, May 5, 2026, at 7:42 AM, Amit Kapila wrote:
> > > > On Tue, May 5, 2026 at 4:02 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >>
> > > >> > Now, we also need to decide whether to backpatch the relevant change
> > > >> > to back-branches. It seems we didn't get the bug-report yet but
> > > >> > clearly what we do currently is not correct. So, we should ideally
> > > >> > backpatch it and in the back branches we don't need to expose it.
> > > >> > OTOH, as it is reported and is not a big issue, so we can keep this as
> > > >> > a HEAD only change as well. If we want to keep this as a HEAD only
> > > >> > change then shall we wait for PG20 branch to open or go for current
> > > >> > HEAD itself? What do you and or others think on this matter?
> > > >>
> > > >> I think we should apply in PG19. Although back-patching isn't
> > > >> critical, since we already have an opportunity to fix it in PG19, why
> > > >> not push it early?
> > > >>
> > > >
> > > > I also think we should push it for PG19 especially because the EXCEPT
> > > > feature increased the usage of relation names without schema-name in
> > > > error messages. However, as we are past feature freeze, I wanted to
> > > > know the opinion of others as well.
> > > >
> > >
> > > -1 for backpatching.
> > >
> >
> > Agreed.
> >
> > > These messages (without schema qualification) has been
> > > like this since the beginning. The function was not introduced by fd366065e06a
> > > and the proposed patch are changing existing messages as well. It is a good
> > > idea to keep visible messages (WARNING, ERROR, FATAL, PANIC) consistent so as
> > > not to break log analysis tools.
> > >
> > > I would say the target is v20. However, as Amit said, the change to the EXCEPT
> > > clause message might be important, so I suggest changing it; I would leave the
> > > other messages for the RMT to decide.
> > >
> >
> > 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.
>
> The updated patch has the changes to split it.
>

Thanks Vignesh.

+/*
+ * get_qualified_relname
+ * Get a palloc'd string containing the schema-qualified name of the relation
+ *  for the given namespace ID and relation name.
+ */
+char *
+get_qualified_relname(Oid nspid, char *relname)

I think, instead of get_qualified_relname(), we should have generic
get_qualified_objname(), so that others can also use it. For example,
see the usage of 'get_namespace_name_or_temp' in
getObjectIdentityParts(), generate_function_name(),
generate_qualified_type_name(),  generate_collation_name() etc, we use
it to get schema-qualified collation names, stats names, dict names,
function name, type-name etc. Making the new function generic will
help most of the other places where we use
get_namespace_name_or_temp().

thanks
Shveta

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-05-07T10:07:54Z

    On Thu, May 7, 2026 at 3:03 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Thu, May 7, 2026 at 11:30 AM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Wed, 6 May 2026 at 09:27, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Tue, May 5, 2026 at 5:56 PM Euler Taveira <euler@eulerto.com> wrote:
> > > >
> > > > On Tue, May 5, 2026, at 7:42 AM, Amit Kapila wrote:
> > > > > On Tue, May 5, 2026 at 4:02 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > > >>
> > > > >> > Now, we also need to decide whether to backpatch the relevant change
> > > > >> > to back-branches. It seems we didn't get the bug-report yet but
> > > > >> > clearly what we do currently is not correct. So, we should ideally
> > > > >> > backpatch it and in the back branches we don't need to expose it.
> > > > >> > OTOH, as it is reported and is not a big issue, so we can keep this as
> > > > >> > a HEAD only change as well. If we want to keep this as a HEAD only
> > > > >> > change then shall we wait for PG20 branch to open or go for current
> > > > >> > HEAD itself? What do you and or others think on this matter?
> > > > >>
> > > > >> I think we should apply in PG19. Although back-patching isn't
> > > > >> critical, since we already have an opportunity to fix it in PG19, why
> > > > >> not push it early?
> > > > >>
> > > > >
> > > > > I also think we should push it for PG19 especially because the EXCEPT
> > > > > feature increased the usage of relation names without schema-name in
> > > > > error messages. However, as we are past feature freeze, I wanted to
> > > > > know the opinion of others as well.
> > > > >
> > > >
> > > > -1 for backpatching.
> > > >
> > >
> > > Agreed.
> > >
> > > > These messages (without schema qualification) has been
> > > > like this since the beginning. The function was not introduced by fd366065e06a
> > > > and the proposed patch are changing existing messages as well. It is a good
> > > > idea to keep visible messages (WARNING, ERROR, FATAL, PANIC) consistent so as
> > > > not to break log analysis tools.
> > > >
> > > > I would say the target is v20. However, as Amit said, the change to the EXCEPT
> > > > clause message might be important, so I suggest changing it; I would leave the
> > > > other messages for the RMT to decide.
> > > >
> > >
> > > 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.
> >
> > The updated patch has the changes to split it.
> >
>
> Thanks Vignesh.
>
> +/*
> + * get_qualified_relname
> + * Get a palloc'd string containing the schema-qualified name of the relation
> + *  for the given namespace ID and relation name.
> + */
> +char *
> +get_qualified_relname(Oid nspid, char *relname)
>
> I think, instead of get_qualified_relname(), we should have generic
> get_qualified_objname(), so that others can also use it.
>

That makes sense. I have changed that in the attached and made slight
modifications in the comments. How about attached?

-- 
With Regards,
Amit Kapila.

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

    Dilip Kumar <dilipbalaut@gmail.com> — 2026-05-07T10:13:55Z

    On Thu, May 7, 2026 at 3:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, May 7, 2026 at 3:03 PM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Thu, May 7, 2026 at 11:30 AM vignesh C <vignesh21@gmail.com> wrote:
> > >
> > > On Wed, 6 May 2026 at 09:27, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > On Tue, May 5, 2026 at 5:56 PM Euler Taveira <euler@eulerto.com> wrote:
> > > > >
> > > > > On Tue, May 5, 2026, at 7:42 AM, Amit Kapila wrote:
> > > > > > On Tue, May 5, 2026 at 4:02 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > > > >>
> > > > > >> > Now, we also need to decide whether to backpatch the relevant change
> > > > > >> > to back-branches. It seems we didn't get the bug-report yet but
> > > > > >> > clearly what we do currently is not correct. So, we should ideally
> > > > > >> > backpatch it and in the back branches we don't need to expose it.
> > > > > >> > OTOH, as it is reported and is not a big issue, so we can keep this as
> > > > > >> > a HEAD only change as well. If we want to keep this as a HEAD only
> > > > > >> > change then shall we wait for PG20 branch to open or go for current
> > > > > >> > HEAD itself? What do you and or others think on this matter?
> > > > > >>
> > > > > >> I think we should apply in PG19. Although back-patching isn't
> > > > > >> critical, since we already have an opportunity to fix it in PG19, why
> > > > > >> not push it early?
> > > > > >>
> > > > > >
> > > > > > I also think we should push it for PG19 especially because the EXCEPT
> > > > > > feature increased the usage of relation names without schema-name in
> > > > > > error messages. However, as we are past feature freeze, I wanted to
> > > > > > know the opinion of others as well.
> > > > > >
> > > > >
> > > > > -1 for backpatching.
> > > > >
> > > >
> > > > Agreed.
> > > >
> > > > > These messages (without schema qualification) has been
> > > > > like this since the beginning. The function was not introduced by fd366065e06a
> > > > > and the proposed patch are changing existing messages as well. It is a good
> > > > > idea to keep visible messages (WARNING, ERROR, FATAL, PANIC) consistent so as
> > > > > not to break log analysis tools.
> > > > >
> > > > > I would say the target is v20. However, as Amit said, the change to the EXCEPT
> > > > > clause message might be important, so I suggest changing it; I would leave the
> > > > > other messages for the RMT to decide.
> > > > >
> > > >
> > > > 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.
> > >
> > > The updated patch has the changes to split it.
> > >
> >
> > Thanks Vignesh.
> >
> > +/*
> > + * get_qualified_relname
> > + * Get a palloc'd string containing the schema-qualified name of the relation
> > + *  for the given namespace ID and relation name.
> > + */
> > +char *
> > +get_qualified_relname(Oid nspid, char *relname)
> >
> > I think, instead of get_qualified_relname(), we should have generic
> > get_qualified_objname(), so that others can also use it.

+1

>
> That makes sense. I have changed that in the attached and made slight
> modifications in the comments. How about attached?

Attached patch LGTM.


-- 
Regards,
Dilip Kumar
Google

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

    shveta malik <shveta.malik@gmail.com> — 2026-05-07T11:46:23Z

    On Thu, May 7, 2026 at 3:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, May 7, 2026 at 3:03 PM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Thu, May 7, 2026 at 11:30 AM vignesh C <vignesh21@gmail.com> wrote:
> > >
> > > On Wed, 6 May 2026 at 09:27, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > > On Tue, May 5, 2026 at 5:56 PM Euler Taveira <euler@eulerto.com> wrote:
> > > > >
> > > > > On Tue, May 5, 2026, at 7:42 AM, Amit Kapila wrote:
> > > > > > On Tue, May 5, 2026 at 4:02 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > > > >>
> > > > > >> > Now, we also need to decide whether to backpatch the relevant change
> > > > > >> > to back-branches. It seems we didn't get the bug-report yet but
> > > > > >> > clearly what we do currently is not correct. So, we should ideally
> > > > > >> > backpatch it and in the back branches we don't need to expose it.
> > > > > >> > OTOH, as it is reported and is not a big issue, so we can keep this as
> > > > > >> > a HEAD only change as well. If we want to keep this as a HEAD only
> > > > > >> > change then shall we wait for PG20 branch to open or go for current
> > > > > >> > HEAD itself? What do you and or others think on this matter?
> > > > > >>
> > > > > >> I think we should apply in PG19. Although back-patching isn't
> > > > > >> critical, since we already have an opportunity to fix it in PG19, why
> > > > > >> not push it early?
> > > > > >>
> > > > > >
> > > > > > I also think we should push it for PG19 especially because the EXCEPT
> > > > > > feature increased the usage of relation names without schema-name in
> > > > > > error messages. However, as we are past feature freeze, I wanted to
> > > > > > know the opinion of others as well.
> > > > > >
> > > > >
> > > > > -1 for backpatching.
> > > > >
> > > >
> > > > Agreed.
> > > >
> > > > > These messages (without schema qualification) has been
> > > > > like this since the beginning. The function was not introduced by fd366065e06a
> > > > > and the proposed patch are changing existing messages as well. It is a good
> > > > > idea to keep visible messages (WARNING, ERROR, FATAL, PANIC) consistent so as
> > > > > not to break log analysis tools.
> > > > >
> > > > > I would say the target is v20. However, as Amit said, the change to the EXCEPT
> > > > > clause message might be important, so I suggest changing it; I would leave the
> > > > > other messages for the RMT to decide.
> > > > >
> > > >
> > > > 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.
> > >
> > > The updated patch has the changes to split it.
> > >
> >
> > Thanks Vignesh.
> >
> > +/*
> > + * get_qualified_relname
> > + * Get a palloc'd string containing the schema-qualified name of the relation
> > + *  for the given namespace ID and relation name.
> > + */
> > +char *
> > +get_qualified_relname(Oid nspid, char *relname)
> >
> > I think, instead of get_qualified_relname(), we should have generic
> > get_qualified_objname(), so that others can also use it.
> >
>
> That makes sense. I have changed that in the attached and made slight
> modifications in the comments. How about attached?
>

LGTM.

Thanks
Shveta

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

    Euler Taveira <euler@eulerto.com> — 2026-05-07T12:19:46Z

    On Thu, May 7, 2026, at 7:07 AM, Amit Kapila wrote:
>
> That makes sense. I have changed that in the attached and made slight
> modifications in the comments. How about attached?
>

One minor suggestion is to rename get_relation_qualified_name to
get_qualified_relation_name. If we want to reuse this new function
get_relation_qualified_name into other places, shouldn't we move this function
to lsyscache.c? There are places like matview.c that could benefit from this new
function.


-- 
Euler Taveira
EDB   https://www.enterprisedb.com/

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

    Euler Taveira <euler@eulerto.com> — 2026-05-07T12:27:09Z

    On Thu, May 7, 2026, at 3:00 AM, vignesh C wrote:
>
> * v5-0001-Include-schema-qualified-names-in-EXCEPT-clause-e.patch
> * v5-0002-Include-schema-qualified-names-in-non-EXCEPT-publ.patch

Regarding v5-0002:

 check_publication_add_relation(PublicationRelInfo *pri)
 {
 	Relation	targetrel = pri->relation;
-	const char *relname;
 	const char *errormsg;
 
 	if (pri->except)
-	{
-		relname = get_relation_qualified_name(targetrel);
 		errormsg = gettext_noop("cannot specify relation \"%s\" in the publication EXCEPT clause");
-	}
 	else
-	{
-		relname = RelationGetRelationName(targetrel);
 		errormsg = gettext_noop("cannot add relation \"%s\" to publication");
-	}

Why did you remove relname variable? In addition to reduce the patch size, you
avoid increasing the number of modifications if the function
get_relation_qualified_name is modified in the future.


-- 
Euler Taveira
EDB   https://www.enterprisedb.com/

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-05-07T12:36:08Z

    On Thu, May 7, 2026 at 5:50 PM Euler Taveira <euler@eulerto.com> wrote:
>
> On Thu, May 7, 2026, at 7:07 AM, Amit Kapila wrote:
> >
> > That makes sense. I have changed that in the attached and made slight
> > modifications in the comments. How about attached?
> >
>
> One minor suggestion is to rename get_relation_qualified_name to
> get_qualified_relation_name. If we want to reuse this new function
> get_relation_qualified_name into other places, shouldn't we move this function
> to lsyscache.c? There are places like matview.c that could benefit from this new
> function.
>

This suggestion sounds reasonable to me but I suggest instead moving
this new function to rel.h as an inline function or into relcache.c as
those are the places where we take functions with Relation as input.
Additionally, we can name the function as
RelationGetQualifiedRelationName().

-- 
With Regards,
Amit Kapila.

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

    Euler Taveira <euler@eulerto.com> — 2026-05-07T12:37:17Z

    On Thu, May 7, 2026, at 9:36 AM, Amit Kapila wrote:
> On Thu, May 7, 2026 at 5:50 PM Euler Taveira <euler@eulerto.com> wrote:
>
> This suggestion sounds reasonable to me but I suggest instead moving
> this new function to rel.h as an inline function or into relcache.c as
> those are the places where we take functions with Relation as input.
> Additionally, we can name the function as
> RelationGetQualifiedRelationName().
>

LGTM.


-- 
Euler Taveira
EDB   https://www.enterprisedb.com/

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

    vignesh C <vignesh21@gmail.com> — 2026-05-08T05:53:50Z

    On Thu, 7 May 2026 at 17:57, Euler Taveira <euler@eulerto.com> wrote:
>
> On Thu, May 7, 2026, at 3:00 AM, vignesh C wrote:
> >
> > * v5-0001-Include-schema-qualified-names-in-EXCEPT-clause-e.patch
> > * v5-0002-Include-schema-qualified-names-in-non-EXCEPT-publ.patch
>
> Regarding v5-0002:
>
>  check_publication_add_relation(PublicationRelInfo *pri)
>  {
>         Relation        targetrel = pri->relation;
> -       const char *relname;
>         const char *errormsg;
>
>         if (pri->except)
> -       {
> -               relname = get_relation_qualified_name(targetrel);
>                 errormsg = gettext_noop("cannot specify relation \"%s\" in the publication EXCEPT clause");
> -       }
>         else
> -       {
> -               relname = RelationGetRelationName(targetrel);
>                 errormsg = gettext_noop("cannot add relation \"%s\" to publication");
> -       }
>
> Why did you remove relname variable? In addition to reduce the patch size, you
> avoid increasing the number of modifications if the function
> get_relation_qualified_name is modified in the future.

I initially wanted to keep the changes aligned with the implementation
in HEAD. However, now that the patch has been committed, I agree with
retaining the variable. The attached patch includes the corresponding
changes.

Regards,
Vignesh

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

    Amit Kapila <amit.kapila16@gmail.com> — 2026-05-08T09:07:45Z

    On Thu, May 7, 2026 at 6:07 PM Euler Taveira <euler@eulerto.com> wrote:
>
> On Thu, May 7, 2026, at 9:36 AM, Amit Kapila wrote:
> > On Thu, May 7, 2026 at 5:50 PM Euler Taveira <euler@eulerto.com> wrote:
> >
> > This suggestion sounds reasonable to me but I suggest instead moving
> > this new function to rel.h as an inline function or into relcache.c as
> > those are the places where we take functions with Relation as input.
> > Additionally, we can name the function as
> > RelationGetQualifiedRelationName().
> >
>
> LGTM.
>

Thanks, I've pushed the patch after making required changes.

-- 
With Regards,
Amit Kapila.

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

    Peter Smith <smithpb2250@gmail.com> — 2026-05-11T07:47:59Z

    I was looking at the code that got pushed [1] for this thread, and I
think there is some quirky quote behaviour:

* The code ends up calling 'quote_qualified_identifier'. That function
does "quoting each component if necessary" (the function name is a
hint!).

* Meanwhile, the error message where the result is substituted already
has hardwired quotes:
errormsg = gettext_noop("cannot specify relation \"%s\" in the
publication EXCEPT clause");

So, nested quoting might occur in the error message.

e.g.
------
test_pub=# CREATE UNLOGGED TABLE "my table"(c int);
CREATE TABLE
test_pub=# CREATE PUBLICATION pub1 FOR ALL TABLES EXCEPT (TABLE "my table");
ERROR:  cannot specify relation "public."my table"" in the publication
EXCEPT clause
DETAIL:  This operation is not supported for unlogged tables.
------

Is that quote nesting OK?

I didn't see this behaviour discussed or tested anywhere, so I cannot
tell whether this was considered already or not.

======
[1] https://github.com/postgres/postgres/commit/a49b9cfd72d89a8fd68c90e84c38aa7e2ada756b

Kind Regards,
Peter Smith.
Fujitsu Australia

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

    vignesh C <vignesh21@gmail.com> — 2026-05-11T11:04:39Z

    On Mon, 11 May 2026 at 13:18, Peter Smith <smithpb2250@gmail.com> wrote:
>
> I was looking at the code that got pushed [1] for this thread, and I
> think there is some quirky quote behaviour:
>
> * The code ends up calling 'quote_qualified_identifier'. That function
> does "quoting each component if necessary" (the function name is a
> hint!).
>
> * Meanwhile, the error message where the result is substituted already
> has hardwired quotes:
> errormsg = gettext_noop("cannot specify relation \"%s\" in the
> publication EXCEPT clause");
>
> So, nested quoting might occur in the error message.
>
> e.g.
> ------
> test_pub=# CREATE UNLOGGED TABLE "my table"(c int);
> CREATE TABLE
> test_pub=# CREATE PUBLICATION pub1 FOR ALL TABLES EXCEPT (TABLE "my table");
> ERROR:  cannot specify relation "public."my table"" in the publication
> EXCEPT clause
> DETAIL:  This operation is not supported for unlogged tables.
> ------
>
> Is that quote nesting OK?

I think this is acceptable, since we already emit similar quoting in
other error messages involving schema qualified identifiers.
For example:
CREATE MATERIALIZED VIEW  "my matv1" as select sum(c1) from t1;

REFRESH MATERIALIZED VIEW concurrently "my matv1";
produces:
ERROR:  cannot refresh materialized view "public."my matv1"" concurrently
HINT:  Create a unique index with no WHERE clause on one or more
columns of the materialized view.

So the current behavior appears to be consistent with existing
PostgreSQL error reporting.

Regards,
Vignesh

  21. 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