On Thu, Sep 17, 2026 at 12:22 PM Peter Smith <[email protected]> wrote: > > On Thu, Sep 17, 2026 at 4:11 PM Amit Kapila <[email protected]> wrote: > > > > On Thu, Sep 17, 2026 at 12:17 AM Manuel Reyes Bravo > > <[email protected]> wrote: > > > > > > vignesh C <[email protected]> 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. > > > > Hi Amit. > > + relname = psprintf("%s.%s", > + get_namespace_name_or_temp(RelationGetNamespace(targetrel)), > + RelationGetRelationName(targetrel)); > > I had been working off-list on a similar patch to this, so I like the > direction where this is heading. > > OTOH please do not remove the RelationGetQualifiedRelationName function. > > Instead, keep the function but replace the body with your > psprint/get_namespace_name_or_temp logic. There are many other places > where the errmsg can benefit from having a common function to output > fully qualified names, but we don't want them all needing to code > their own psprintf/get_namespace_name_or_temp. >
Even if we go in that direction, it should be for PG20. In PG19, we can't extend its usage. So, I prefer to drop it and if we can come up with a use case for it that is acceptable then we can consider it again. -- With Regards, Amit Kapila.
