| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Manuel Reyes Bravo <manuelreyesbravo(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(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>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Distinguish publication exclusions in object addresses |
| Date: | 2026-09-17 06:51:35 |
| Message-ID: | CAJpy0uCJ4FHzQV75hdpJwNQkutX0frLjqfdTh9OETsKfWgsXdg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Sep 17, 2026 at 11:41 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Sep 17, 2026 at 12:17 AM Manuel Reyes Bravo
> <manuelreyesbravo(at)gmail(dot)com> wrote:
> >
> > vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > > Please have a look at the discussion regarding the commit in [1]
> > > thread. If you still feel that it needs further discussion, we can
> > > continue discussing your suggestion in the thread at [2], as it is not
> > > directly related to the topic of this current thread.
> >
> > Thanks, I had not seen [2]. Peter raised the nested quotes there on
> > May 11 with the same kind of example, and you considered them
> > acceptable, citing REFRESH MATERIALIZED VIEW CONCURRENTLY. So 0002 is
> > not a new finding, and I withdraw it.
> >
> > I also need to correct a number I gave. I wrote that 55 errmsg() calls
> > in src/backend use "\"%s.%s\"" against 3 that put a quoted qualified
> > name inside quotes. My search missed calls whose arguments are on
> > later lines. Reading each errmsg/errdetail/errhint call in full on
> > REL_19_STABLE, it is 70 against 2: matview.c (the case you cited) and
> > the EXCEPT message in pg_publication.c.
> >
>
> Seeing the other usage, I am thinking to fix this case for except
> tables unless some major change is required. We can consider changing
> REFRESH MATERIALIZED VIEW CONCURRENTLY separately. I have few points
> for the that patch:
>
> * The existing function
> RelationGetQualifiedRelationName()->get_qualified_objname() used
> get_namespace_name_or_temp() where as patch used get_namespace_name().
> That could create the problem with temp tables as follows:
>
> Before patch:
> postgres=# create publication mypub for all tables except(table mytemp);
> ERROR: cannot specify relation "pg_temp.mytemp" in the publication
> EXCEPT clause
> DETAIL: This operation is not supported for temporary tables.
>
> After patch:
> postgres=# create publication mypub for all tables except(table mytemp);
> ERROR: cannot specify relation "pg_temp_0.mytemp" in the publication
> EXCEPT clause
> DETAIL: This operation is not supported for temporary tables.
>
> You can notice that temp schema name has _0 appended to it after
> patch, it seems to append backend's slot number. We can use
> get_namespace_name_or_temp() instead of get_namespace_name().
>
> * After this RelationGetQualifiedRelationName() is dead and is
> introduced by the commit a49b9cfd72d89a8fd68c90e84c38aa7e2ada756b
> which introduced this quoting problem. I feel it is better to remove
> it.
>
> Attached fixed both the problems.
The patch LGTM.
Another option would be to drop the hardcoded quotes from errormsg and
keep RelationGetQualifiedRelationName, but maintaining a consistent
error-messaging style across similar catalog checks is preferable.
Thus, the current patch looks good as-is.
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Smith | 2026-09-17 06:52:02 | Re: Distinguish publication exclusions in object addresses |
| Previous Message | Rui Zhao | 2026-09-17 06:51:04 | Re: Persist slot invalidations before publishing them |