| From: | Manuel Reyes Bravo <manuelreyesbravo(at)gmail(dot)com> |
|---|---|
| To: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
| Cc: | shveta malik <shveta(dot)malik(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, vignesh C <vignesh21(at)gmail(dot)com> |
| Subject: | Re: Distinguish publication exclusions in object addresses |
| Date: | 2026-09-16 12:42:03 |
| Message-ID: | CA+bCEdC7dU-+v29vK9bpZhHJs=CHvA-EQ-ThxnC1TCDqfwK7VQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> I agree that there is no confusion in above case. But I also don't see
> any harm in having a schema-qualified name. It makes the message
> clearer. The cases like below can become more understandable with
> schema qualified name.
[...]
> postgres=# SET search_path = s2;
> postgres=# SELECT pg_get_object_address('publication relation',
> '{t2}', '{pub}');
> ERROR: publication relation "t2" in publication "pub" does not exist
Zhijie is right that my own example was weak: the schema was right there
in the call. Your example is the one that matters, and it goes through
the "does not exist" message, which my first delta did not touch. The
attached 0001 now qualifies all four errors in
get_object_address_publication_rel(), so that case reads:
ERROR: publication relation "s2.t2" in publication "pub" does not exist
> If at all we plan to add it, it will be good to use
> 'RelationGetQualifiedRelationName' instead.
I tried it first, and it quotes the name twice when it needs quoting,
because it goes through quote_qualified_identifier() and these messages
already put the name in quotes:
ERROR: publication relation ""Sales Q3"."Big Table"" in
publication "pub" does not exist
So 0001 uses "\"%s.%s\"" with get_namespace_name(), which is the form of
55 errmsg() calls in src/backend, against 3 that put a quoted qualified
name inside quotes.
That also shows the same problem in commit a49b9cfd72d, which is in
REL_19_STABLE:
CREATE TABLE "testpub Part2" PARTITION OF testpub_root FOR VALUES
FROM (100) TO (200);
CREATE PUBLICATION testpub9 FOR ALL TABLES EXCEPT (TABLE "testpub Part2");
ERROR: cannot specify relation "public."testpub Part2"" in the
publication EXCEPT clause
0002 fixes it by building the name the same way, without changing the
message text, and adds that case to publication.sql. Without the fix
the new test fails with the output above; with it, it passes. 0002 does
not depend on v5 and applies to both master and REL_19_STABLE.
Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> There is no harm, but it is not consistent with the other object
> messages used here (e.g., they do not report schema-qualified names):
That is true of objectaddress.c: none of its messages qualifies the name
today. The case for doing it here is the one a49b9cfd72d made for the
EXCEPT errors, that the same name can exist in several schemas, plus
shveta's search_path example. Whether that outweighs consistency with
the rest of objectaddress.c is Amit's call; if not, 0001 can simply be
dropped, and 0002 still stands on its own.
v5 plus 0001 and 0002: all 239 regression tests pass.
> Just a kind suggestion: the community prefers inline replies rather than
> top-posting
Thanks, noted.
Regards,
Manu
El mié, 16 sept 2026 a las 7:23, Zhijie Hou (Fujitsu)
(<houzj(dot)fnst(at)fujitsu(dot)com>) escribió:
>
> Hi,
>
> On Wednesday, September 16, 2026 6:14 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > On Wed, Sep 16, 2026 at 3:24 PM Zhijie Hou (Fujitsu)
> > > On Wednesday, September 16, 2026 5:20 PM Manuel Reyes Bravo
> > <manuelreyesbravo(at)gmail(dot)com> wrote:
> > > > CREATE TABLE t2(a int);
> > > > CREATE SCHEMA s2; CREATE TABLE s2.t2(a int);
> > > > CREATE PUBLICATION pub FOR ALL TABLES EXCEPT (TABLE s2.t2);
> > > >
> > > > SELECT pg_get_object_address('publication relation','{s2,t2}','{pub}');
> > > > ERROR: "t2" is not a published relation of publication "pub"
> > > >
> > > > So the message says that "t2" is not a published relation of pub, while
> > > > the catalog says that a t2 is. They are different tables, and the user
> > > > cannot tell which one the message is about.
> > >
> > > In this case, the user is explicitly passing s2.t2 as a parameter to get its
> > > object address. In that context, it's pretty clear to me that t2 in the message
> > > refers exactly to the object the user passed, I personally don't think it could
> > > point to any other table.
> >
> > I agree that there is no confusion in above case. But I also don't see
> > any harm in having a schema-qualified name. It makes the message
> > clearer. The cases like below can become more understandable with
> > schema qualified name.
>
> There is no harm, but it is not consistent with the other object
> messages used here (e.g., they do not report schema-qualified names):
>
> get_relation_by_qualified_name
> get_object_address_attribute
> get_object_address_attrdef
> get_object_address_unqualified
> get_object_address_type
> ...
>
> So it does not look necessary to me to add it for our publication case.
>
> The column list error message reports schema-qualified names because users can
> specify multiple tables with the same name from different schemas, so it's
> necessary there. But that's not our case.
>
> Best Regards,
> Zhijie Hou
| Attachment | Content-Type | Size |
|---|---|---|
| v5-delta-v2-0002-Don-t-quote-the-relation-name-twice-in-EXCEPT-clause.patch | text/x-patch | 4.5 KB |
| v5-delta-v2-0001-Qualify-the-relation-name-in-publication-relation-ob.patch | text/x-patch | 4.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Daniel Gustafsson | 2026-09-16 12:43:15 | Re: pgsql: Revert online data checksum transitions |
| Previous Message | Aleksander Alekseev | 2026-09-16 12:32:07 | Re: pgsql: Revert online data checksum transitions |