Horton

Thread

Commits

  1. Fix issues with describe queries of extended statistics in psql

  2. Properly schema-prefix reference to pg_catalog.pg_get_statisticsobjdef_columns

  3. psql \dX: reference regclass with "pg_catalog." prefix

  1. Add prefix pg_catalog to pg_get_statisticsobjdef_columns() in describe.c (\dX)

    Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp> — 2021-09-27T00:13:19Z

    Hi,

I found other functions that we should add "pg_catalog" prefix in
describe.c. This fix is similar to the following commit.

=====
commit 359bcf775550aa577c86ea30a6d071487fcca1ed
Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date:   Sat Aug 28 12:04:15 2021 -0400

     psql \dX: reference regclass with "pg_catalog." prefix
=====

Please find attached the patch.

Regards,
Tatsuro Yamada

  2. Re: Add prefix pg_catalog to pg_get_statisticsobjdef_columns() in describe.c (\dX)

    Magnus Hagander <magnus@hagander.net> — 2021-09-28T14:25:11Z

    On Mon, Sep 27, 2021 at 2:14 AM Tatsuro Yamada <
tatsuro.yamada.tf@nttcom.co.jp> wrote:

> Hi,
>
> I found other functions that we should add "pg_catalog" prefix in
> describe.c. This fix is similar to the following commit.
>

Hi!

Yup, that's correct. Applied and backpatched to 14 (but it won't be in the
14.0 release).

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

  3. Re: Add prefix pg_catalog to pg_get_statisticsobjdef_columns() in describe.c (\dX)

    Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp> — 2021-09-28T23:54:18Z

    Hi Magnus!


>     I found other functions that we should add "pg_catalog" prefix in
>     describe.c. This fix is similar to the following commit.
> 
> Yup, that's correct. Applied and backpatched to 14 (but it won't be in the 14.0 release).


Thank you!


Regards,
Tatsuro Yamada

  4. Re: \dP and \dX use ::regclass without "pg_catalog."

    Justin Pryzby <pryzby@telsasoft.com> — 2022-01-07T02:22:35Z

    https://www.postgresql.org/message-id/flat/7ad8cd13-db5b-5cf6-8561-dccad1a934cb%40nttcom.co.jp
https://www.postgresql.org/message-id/flat/20210827193151.GN26465%40telsasoft.com

On Sat, Aug 28, 2021 at 08:57:32AM -0400, Álvaro Herrera wrote:
> On 2021-Aug-27, Justin Pryzby wrote:
> > I noticed that for \dP+ since 1c5d9270e, regclass is written without
> > "pg_catalog." (Alvaro and I failed to notice it in 421a2c483, too).
> 
> Oops, will fix shortly.

On Tue, Sep 28, 2021 at 04:25:11PM +0200, Magnus Hagander wrote:
> On Mon, Sep 27, 2021 at 2:14 AM Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp> wrote:
> > I found other functions that we should add "pg_catalog" prefix in
> > describe.c. This fix is similar to the following commit.
> 
> Yup, that's correct. Applied and backpatched to 14 (but it won't be in the
> 14.0 release).

Those two issues were fixed in 1f092a309 and 07f8a9e784.
But, we missed two casts to ::text which don't use pg_catalog.
Evidently the cast is to allow stable sorting.

I improved on my previous hueristic to look for these ; this finds the two
missing schema qualifiers:

> time grep "$(sed -r "/.*pg_catalog\\.([_[:alpha:]]+).*/! d; s//\\1/; /^(char|oid)$/d; s/.*/[^. ']\\\\<&\\\\>/" src/bin/psql/describe.c |sort -u)" src/bin/psql/describe.c

While looking at that, I wondered why describeOneTableDetails lists stats
objects in order of OID ?  Dating back to 7b504eb28, and, before that,
554A73A6.2060603@2ndquadrant.com.  It should probably order by nsp, stxname.

  5. Re: \dP and \dX use ::regclass without "pg_catalog."

    Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp> — 2022-01-07T09:30:30Z

    Hi Justin,

On 2022/01/07 11:22, Justin Pryzby wrote:
> But, we missed two casts to ::text which don't use pg_catalog.
> Evidently the cast is to allow stable sorting.

Ah, you are right.

We should prefix them with pg_catalog as well.
Are you planning to make a patch?
If not, I'll make a patch later since that's where \dX is.

Regards,
Tatsuro Yamada

  6. Re: \dP and \dX use ::regclass without "pg_catalog."

    Michael Paquier <michael@paquier.xyz> — 2022-01-07T11:08:57Z

    On Fri, Jan 07, 2022 at 06:30:30PM +0900, Tatsuro Yamada wrote:
> We should prefix them with pg_catalog as well.
> Are you planning to make a patch?
> If not, I'll make a patch later since that's where \dX is.

If any of you can make a patch, that would be great.  Thanks!
--
Michael

  7. Re: \dP and \dX use ::regclass without "pg_catalog."

    Justin Pryzby <pryzby@telsasoft.com> — 2022-01-07T21:56:20Z

    On Fri, Jan 07, 2022 at 08:08:57PM +0900, Michael Paquier wrote:
> On Fri, Jan 07, 2022 at 06:30:30PM +0900, Tatsuro Yamada wrote:
> > We should prefix them with pg_catalog as well.
> > Are you planning to make a patch?
> > If not, I'll make a patch later since that's where \dX is.
> 
> If any of you can make a patch, that would be great.  Thanks!

I'd propose these.

  8. Re: \dP and \dX use ::regclass without "pg_catalog."

    Michael Paquier <michael@paquier.xyz> — 2022-01-08T07:47:52Z

    On Fri, Jan 07, 2022 at 03:56:20PM -0600, Justin Pryzby wrote:
> I'd propose these.

Applied and backpatched down to 14.  One of the aliases is present in
13~, but I have let that alone.  The detection regex posted upthread
is kind of cool.  
--
Michael

  9. Re: \dP and \dX use ::regclass without "pg_catalog."

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-01-08T19:50:19Z

    On 2022-Jan-08, Michael Paquier wrote:

> The detection regex posted upthread is kind of cool.  

Yes, but it's not bulletproof -- it only detects uses of some
unqualified object name that is also used with qualification.  Here it
detected "text" unqualified, but only because we already had
pg_catalog.text elsewhere.  As an exercise, if you revert this commit
and change one of those "text" to "int", it's not detected as a problem.

My point is that it's good to have it, but it would be much better to
have something bulletproof, which we could use in an automated check
somewhere (next to stuff like perlcritic, perhaps).  I don't know what,
though.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Cuando no hay humildad las personas se degradan" (A. Christie)

  10. Re: \dP and \dX use ::regclass without "pg_catalog."

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-01-08T20:03:39Z

    =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@alvh.no-ip.org> writes:
> My point is that it's good to have it, but it would be much better to
> have something bulletproof, which we could use in an automated check
> somewhere (next to stuff like perlcritic, perhaps).  I don't know what,
> though.

Meh ... this is mostly cosmetic these days, so I can't get excited
about putting a lot of work into it.  We disclaimed search path
bulletproofness for all these queries a long time ago.

I don't object to fixing it in the name of consistency, but that's
not a reason to invest large effort.

			regards, tom lane

  11. Re: \dP and \dX use ::regclass without "pg_catalog."

    Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp> — 2022-01-11T01:57:29Z

    Hi justin,

On 2022/01/08 6:56, Justin Pryzby wrote:
> On Fri, Jan 07, 2022 at 08:08:57PM +0900, Michael Paquier wrote:
>> On Fri, Jan 07, 2022 at 06:30:30PM +0900, Tatsuro Yamada wrote:
>>> We should prefix them with pg_catalog as well.
>>> Are you planning to make a patch?
>>> If not, I'll make a patch later since that's where \dX is.
>>
>> If any of you can make a patch, that would be great.  Thanks!
> 
> I'd propose these.


Thanks!

Regards,
Tatsuro Yamada