From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | jian he <jian(dot)universality(at)gmail(dot)com>, John Naylor <johncnaylorls(at)gmail(dot)com> |
Cc: | Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Steve Chavez <steve(at)supabase(dot)io>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Add pg_basetype() function to obtain a DOMAIN base type |
Date: | 2024-02-16 18:16:18 |
Message-ID: | 53392389-7620-46af-889e-cd1938a53afa@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 1/2/24 01:00, jian he wrote:
> On Mon, Dec 4, 2023 at 5:11 PM John Naylor <johncnaylorls(at)gmail(dot)com> wrote:
>>
>> On Thu, Sep 28, 2023 at 12:22 AM Alexander Korotkov
>> <aekorotkov(at)gmail(dot)com> wrote:
>>> The one thing triggering my perfectionism is that the patch does two
>>> syscache lookups instead of one.
>>
>> For an admin function used interactively, I'm not sure why that
>> matters? Or do you see another use case?
>
> I did a minor refactor based on v1-0001.
> I think pg_basetype should stay at "9.26.4. System Catalog Information
> Functions".
> So I placed it before pg_char_to_encoding.
> Now functions listed on "Table 9.73. System Catalog Information
> Functions" will look like alphabetical ordering.
> I slightly changed the src/include/catalog/pg_proc.dat.
> now it looks like very similar to pg_typeof
>
> src6=# \df pg_typeof
> List of functions
> Schema | Name | Result data type | Argument data types | Type
> ------------+-----------+------------------+---------------------+------
> pg_catalog | pg_typeof | regtype | "any" | func
> (1 row)
>
> src6=# \df pg_basetype
> List of functions
> Schema | Name | Result data type | Argument data types | Type
> ------------+-------------+------------------+---------------------+------
> pg_catalog | pg_basetype | regtype | "any" | func
> (1 row)
>
> v2-0001 is as is in the first email thread, 0002 is my changes based on v2-0001.
I think the patch(es) look reasonable, so just a couple minor comments.
1) We already have pg_typeof() function, so maybe we should use a
similar naming convention pg_basetypeof()?
2) I was going to suggest using "any" argument, just like pg_typeof, but
I see 0002 patch already does that. Thanks!
3) I think the docs probably need some formatting - wrapping lines (to
make it consistent with the nearby stuff) and similar stuff.
Other than that it looks fine to me. It's a simple patch, so if we can
agree on the naming I'll get it cleaned up and pushed.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2024-02-16 18:24:16 | Re: table inheritance versus column compression and storage settings |
Previous Message | Tomas Vondra | 2024-02-16 17:50:19 | Re: Extend pgbench partitioning to pgbench_history |