Обсуждение: Add pg_basetype() function to obtain a DOMAIN base type

Поиск
Список
Период
Сортировка

Add pg_basetype() function to obtain a DOMAIN base type

От
Steve Chavez
Дата:
Hello hackers,

Currently obtaining the base type of a domain involves a somewhat long recursive query. Consider:

```
create domain mytext as text;
create domain mytext_child_1 as mytext;
create domain mytext_child_2 as mytext_child_1;
```

To get `mytext_child_2` base type we can do:

```
WITH RECURSIVE
recurse AS (
  SELECT
    oid,
    typbasetype,
    COALESCE(NULLIF(typbasetype, 0), oid) AS base
  FROM pg_type
  UNION
  SELECT
    t.oid,
    b.typbasetype,
    COALESCE(NULLIF(b.typbasetype, 0), b.oid) AS base
  FROM recurse t
  JOIN pg_type b ON t.typbasetype = b.oid
)
SELECT
  oid::regtype,
  base::regtype
FROM recurse
WHERE typbasetype = 0 and oid = 'mytext_child_2'::regtype;

      oid       | base
----------------+------
 mytext_child_2 | text
```

Core has the `getBaseType` function, which already gets a domain base type recursively.

I've attached a patch that exposes a `pg_basetype` SQL function that uses `getBaseType`, so the long query above just becomes:

```
select pg_basetype('mytext_child_2'::regtype);
 pg_basetype
-------------
 text
(1 row)
```

Tests and docs are added.

Best regards,
Steve Chavez
Вложения

Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Steve Chavez
Дата:
Just to give a data point for the need of this function:


This is also a common use case for services/extensions that require postgres metadata for their correct functioning, like postgREST or pg_graphql.

Here's a query for getting domain base types, taken from the postgREST codebase:

So having `pg_basetype` would be really helpful in those cases.

Looking forward to hearing any feedback. Or if this would be a bad idea.

Best regards,
Steve Chavez

On Sat, 9 Sept 2023 at 01:17, Steve Chavez <steve@supabase.io> wrote:
Hello hackers,

Currently obtaining the base type of a domain involves a somewhat long recursive query. Consider:

```
create domain mytext as text;
create domain mytext_child_1 as mytext;
create domain mytext_child_2 as mytext_child_1;
```

To get `mytext_child_2` base type we can do:

```
WITH RECURSIVE
recurse AS (
  SELECT
    oid,
    typbasetype,
    COALESCE(NULLIF(typbasetype, 0), oid) AS base
  FROM pg_type
  UNION
  SELECT
    t.oid,
    b.typbasetype,
    COALESCE(NULLIF(b.typbasetype, 0), b.oid) AS base
  FROM recurse t
  JOIN pg_type b ON t.typbasetype = b.oid
)
SELECT
  oid::regtype,
  base::regtype
FROM recurse
WHERE typbasetype = 0 and oid = 'mytext_child_2'::regtype;

      oid       | base
----------------+------
 mytext_child_2 | text
```

Core has the `getBaseType` function, which already gets a domain base type recursively.

I've attached a patch that exposes a `pg_basetype` SQL function that uses `getBaseType`, so the long query above just becomes:

```
select pg_basetype('mytext_child_2'::regtype);
 pg_basetype
-------------
 text
(1 row)
```

Tests and docs are added.

Best regards,
Steve Chavez

Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Alexander Korotkov
Дата:
Hi, Steve!

On Tue, Sep 19, 2023 at 8:36 PM Steve Chavez <steve@supabase.io> wrote:
>
> Just to give a data point for the need of this function:
>
> https://dba.stackexchange.com/questions/231879/how-to-get-the-basetype-of-a-domain-in-pg-type
>
> This is also a common use case for services/extensions that require postgres metadata for their correct functioning,
likepostgREST or pg_graphql. 
>
> Here's a query for getting domain base types, taken from the postgREST codebase:
>
https://github.com/PostgREST/postgrest/blob/531a183b44b36614224fda432335cdaa356b4a0a/src/PostgREST/SchemaCache.hs#L342-L364
>
> So having `pg_basetype` would be really helpful in those cases.
>
> Looking forward to hearing any feedback. Or if this would be a bad idea.

I think this is a good idea.  It's nice to have a simple (and fast)
built-in function to call instead of investing complex queries over
the system catalog.

The one thing triggering my perfectionism is that the patch does two
syscache lookups instead of one.  In order to fit into one syscache
lookup we could add "bool missing_ok" argument to
getBaseTypeAndTypmod().  However, getBaseTypeAndTypmod() is heavily
used in our codebase.  So, changing its signature would be invasive.
Could we invent getBaseTypeAndTypmodExtended() (ideas for a better
name?) that does all the job and supports "bool missing_ok" argument,
and have getBaseTypeAndTypmod() as a wrapper with the same signature?

------
Regards,
Alexander Korotkov



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
jian he
Дата:
On Thu, Sep 28, 2023 at 11:56 AM Alexander Korotkov
<aekorotkov@gmail.com> wrote:
>
> The one thing triggering my perfectionism is that the patch does two
> syscache lookups instead of one.  In order to fit into one syscache
> lookup we could add "bool missing_ok" argument to
> getBaseTypeAndTypmod().  However, getBaseTypeAndTypmod() is heavily
> used in our codebase.  So, changing its signature would be invasive.
> Could we invent getBaseTypeAndTypmodExtended() (ideas for a better
> name?) that does all the job and supports "bool missing_ok" argument,
> and have getBaseTypeAndTypmod() as a wrapper with the same signature?
>

hi.
attached patch, not 100% confident it's totally correct, but one
syscache lookup.
another function getBaseTypeAndTypmodExtended added.

getBaseTypeAndTypmodExtended function signature:
Oid getBaseTypeAndTypmodExtended(Oid typid, int32 *typmod, bool missing_ok).

based on Steve Chavez's patch, minor doc changes.

Вложения

Re: Add pg_basetype() function to obtain a DOMAIN base type

От
John Naylor
Дата:
On Thu, Sep 28, 2023 at 12:22 AM Alexander Korotkov
<aekorotkov@gmail.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?



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
jian he
Дата:
On Mon, Dec 4, 2023 at 5:11 PM John Naylor <johncnaylorls@gmail.com> wrote:
>
> On Thu, Sep 28, 2023 at 12:22 AM Alexander Korotkov
> <aekorotkov@gmail.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.

Вложения

Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Tomas Vondra
Дата:
Hi,

On 1/2/24 01:00, jian he wrote:
> On Mon, Dec 4, 2023 at 5:11 PM John Naylor <johncnaylorls@gmail.com> wrote:
>>
>> On Thu, Sep 28, 2023 at 12:22 AM Alexander Korotkov
>> <aekorotkov@gmail.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



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
jian he
Дата:
On Sat, Feb 17, 2024 at 2:16 AM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
> Hi,
>
> On 1/2/24 01:00, jian he wrote:
> > On Mon, Dec 4, 2023 at 5:11 PM John Naylor <johncnaylorls@gmail.com> wrote:
> >>
> >> On Thu, Sep 28, 2023 at 12:22 AM Alexander Korotkov
> >> <aekorotkov@gmail.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()?
>
I am ok with pg_basetypeof.



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Tomas Vondra
Дата:

On 2/17/24 01:57, jian he wrote:
> On Sat, Feb 17, 2024 at 2:16 AM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
>>
>> Hi,
>>
>> On 1/2/24 01:00, jian he wrote:
>>> On Mon, Dec 4, 2023 at 5:11 PM John Naylor <johncnaylorls@gmail.com> wrote:
>>>>
>>>> On Thu, Sep 28, 2023 at 12:22 AM Alexander Korotkov
>>>> <aekorotkov@gmail.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()?
>>
> I am ok with pg_basetypeof.

An alternative approach would be modifying pg_typeof() to optionally
determine the base type, depending on a new argument which would default
to "false" (i.e. the current behavior).

So you'd do

  SELECT pg_typeof(x);

or

  SELECT pg_typeof(x, false);

to get the current behavior, or and

  SELECT pg_typeof(x, true);

to determine the base type.


Perhaps this would be better than adding a new function doing almost the
same thing as pg_typeof(). But I haven't tried, maybe it doesn't work
for some reason, or maybe we don't want to do it this way ...


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Tom Lane
Дата:
Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
> On 2/17/24 01:57, jian he wrote:
>> On Sat, Feb 17, 2024 at 2:16 AM Tomas Vondra
>> <tomas.vondra@enterprisedb.com> wrote:
>>> 1) We already have pg_typeof() function, so maybe we should use a
>>> similar naming convention pg_basetypeof()?

>> I am ok with pg_basetypeof.

> An alternative approach would be modifying pg_typeof() to optionally
> determine the base type, depending on a new argument which would default
> to "false" (i.e. the current behavior).

Forgive me for not having read the thread, but I wonder why we want
this to duplicate the functionality of pg_typeof() at all.  My first
reaction to the requirement given in the thread subject is to write
a function that takes a type OID and returns another type OID
(or the same OID, if it's not a domain).  If you want to determine
the base type of some namable object, you could combine the functions
like "basetypeof(pg_typeof(x))".  But ISTM there are other use cases
where you'd have a type OID.  Then having to construct an object to
apply a pg_typeof-like function to would be difficult.

I don't have an immediate proposal for exactly what to call such a
function, but naming it by analogy to pg_typeof would be questionable.

            regards, tom lane



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Tomas Vondra
Дата:

On 2/17/24 20:20, Tom Lane wrote:
> Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
>> On 2/17/24 01:57, jian he wrote:
>>> On Sat, Feb 17, 2024 at 2:16 AM Tomas Vondra
>>> <tomas.vondra@enterprisedb.com> wrote:
>>>> 1) We already have pg_typeof() function, so maybe we should use a
>>>> similar naming convention pg_basetypeof()?
> 
>>> I am ok with pg_basetypeof.
> 
>> An alternative approach would be modifying pg_typeof() to optionally
>> determine the base type, depending on a new argument which would default
>> to "false" (i.e. the current behavior).
> 
> Forgive me for not having read the thread, but I wonder why we want
> this to duplicate the functionality of pg_typeof() at all.  My first
> reaction to the requirement given in the thread subject is to write
> a function that takes a type OID and returns another type OID
> (or the same OID, if it's not a domain).  If you want to determine
> the base type of some namable object, you could combine the functions
> like "basetypeof(pg_typeof(x))".  But ISTM there are other use cases
> where you'd have a type OID.  Then having to construct an object to
> apply a pg_typeof-like function to would be difficult.
> 

Yeah, I think you're right - the initial message does actually seem to
indicate it needs to pass type "type OID" to the function, not some
arbitrary expression (and then process a type of it). So modeling it per
pg_typeof(any) would not even work.

Also, now that I looked at the v2 patch again, I see it only really
tweaked the pg_proc.dat entry, but the code still does PG_GETARG_OID (so
the "any" bit is not really correct).

> I don't have an immediate proposal for exactly what to call such a
> function, but naming it by analogy to pg_typeof would be questionable.
> 

Are you objecting to the pg_basetypeof() name, or just to it accepting
"any" argument? I think pg_basetypeof(regtype) would work ...


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Tom Lane
Дата:
Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
> On 2/17/24 20:20, Tom Lane wrote:
>> I don't have an immediate proposal for exactly what to call such a
>> function, but naming it by analogy to pg_typeof would be questionable.

> Are you objecting to the pg_basetypeof() name, or just to it accepting
> "any" argument? I think pg_basetypeof(regtype) would work ...

I'm not sure.  "pg_basetypeof" seems like it invites confusion with
"pg_typeof", but I don't really have a better idea.  Perhaps
"pg_baseofdomain(regtype)"?  I'm not especially thrilled with that,
either.

Also, just to be clear, we intend this to drill down to the bottom
non-domain type, right?  Do we need a second function that goes
down only one level?  I'm inclined to say "no", mainly because
(1) that would complicate the naming situation even more, and
(2) that use-case is pretty easy to handle with a sub-select.

            regards, tom lane



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
jian he
Дата:
On Sun, Feb 18, 2024 at 2:49 AM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
> An alternative approach would be modifying pg_typeof() to optionally
> determine the base type, depending on a new argument which would default
> to "false" (i.e. the current behavior).
>
> So you'd do
>
>   SELECT pg_typeof(x);
>
> or
>
>   SELECT pg_typeof(x, false);
>
> to get the current behavior, or and
>
>   SELECT pg_typeof(x, true);
>
> to determine the base type.
>
> Perhaps this would be better than adding a new function doing almost the
> same thing as pg_typeof(). But I haven't tried, maybe it doesn't work
> for some reason, or maybe we don't want to do it this way ...
>

pg_typeof is quite hot.
getting the base type of a domain is niche.

changing pg_typeof requires extra effort to make it compatible with
previous behavior.
bundling it together seems not worth it.



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
jian he
Дата:
On Sun, Feb 18, 2024 at 7:29 AM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
>
> Also, now that I looked at the v2 patch again, I see it only really
> tweaked the pg_proc.dat entry, but the code still does PG_GETARG_OID (so
> the "any" bit is not really correct).
>

PG_GETARG_OID part indeed is wrong. so I change to following:

+Datum
+pg_basetype(PG_FUNCTION_ARGS)
+{
+ Oid oid;
+
+ oid =  get_fn_expr_argtype(fcinfo->flinfo, 0);
+ if (!SearchSysCacheExists1(TYPEOID, ObjectIdGetDatum(oid)))
+ PG_RETURN_NULL();
+
+ PG_RETURN_OID(getBaseType(oid));
+}

I still name the function as pg_basetype, feel free to change it.

+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>pg_basetype</primary>
+        </indexterm>
+        <function>pg_basetype</function> ( <type>"any"</type> )
+        <returnvalue>regtype</returnvalue>
+       </para>
+       <para>
+       Returns the OID of the base type of a domain or if the
argument is a basetype it returns the same type.
+       If there's a chain of domain dependencies, it will recurse
until finding the base type.
+       </para>
compared with pg_typeof's explanation, I feel like pg_basetype's
explanation doesn't seem accurate.
However, I don't know how to rephrase it.

Вложения

Re: Add pg_basetype() function to obtain a DOMAIN base type

От
jian he
Дата:
looking at it again.
I found out we can just simply do
`
Datum
pg_basetype(PG_FUNCTION_ARGS)
{
Oid oid;

oid =  get_fn_expr_argtype(fcinfo->flinfo, 0);
PG_RETURN_OID(getBaseType(oid));
}
`

if the type is not a domain, work the same as  pg_typeof.
if the type is domain,  pg_typeof return as is, pg_basetype return the
base type.
so it only diverges when the argument type is a type of domain.

the doc:
        <function>pg_basetype</function> ( <type>"any"</type> )
        <returnvalue>regtype</returnvalue>
       </para>
       <para>
       Returns the OID of the base type of a domain. If the argument
is not a type of domain,
       return the OID of the data type of the argument just like <link
linkend="function-pg-typeof"><function>pg_typeof()</function></link>.
       If there's a chain of domain dependencies, it will recurse
until finding the base type.
       </para>


also, I think this way, we only do one syscache lookup.

Вложения

Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Alexander Korotkov
Дата:
On Mon, Mar 18, 2024 at 2:01 AM jian he <jian.universality@gmail.com> wrote:
>
> looking at it again.
> I found out we can just simply do
> `
> Datum
> pg_basetype(PG_FUNCTION_ARGS)
> {
> Oid oid;
>
> oid =  get_fn_expr_argtype(fcinfo->flinfo, 0);
> PG_RETURN_OID(getBaseType(oid));
> }
> `
>
> if the type is not a domain, work the same as  pg_typeof.
> if the type is domain,  pg_typeof return as is, pg_basetype return the
> base type.
> so it only diverges when the argument type is a type of domain.
>
> the doc:
>         <function>pg_basetype</function> ( <type>"any"</type> )
>         <returnvalue>regtype</returnvalue>
>        </para>
>        <para>
>        Returns the OID of the base type of a domain. If the argument
> is not a type of domain,
>        return the OID of the data type of the argument just like <link
> linkend="function-pg-typeof"><function>pg_typeof()</function></link>.
>        If there's a chain of domain dependencies, it will recurse
> until finding the base type.
>        </para>
>
>
> also, I think this way, we only do one syscache lookup.

Looks good to me.  But should it be named pg_basetypeof()?

------
Regards,
Alexander Korotkov



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Tom Lane
Дата:
Alexander Korotkov <aekorotkov@gmail.com> writes:
> On Mon, Mar 18, 2024 at 2:01 AM jian he <jian.universality@gmail.com> wrote:
>> `
>> Datum
>> pg_basetype(PG_FUNCTION_ARGS)
>> {
>>     Oid oid;
>>
>>     oid =  get_fn_expr_argtype(fcinfo->flinfo, 0);
>>     PG_RETURN_OID(getBaseType(oid));
>> }
>> `

> Looks good to me.  But should it be named pg_basetypeof()?

I still don't like this approach.  It forces the function to be
used in a particular way that's highly redundant with pg_typeof.
I think we'd be better off with

pg_basetype(PG_FUNCTION_ARGS)
{
    Oid typid = PG_GETARG_OID(0);

    PG_RETURN_OID(getBaseType(typid));
}

The use-case that the other definition handles would be implemented
like

    pg_basetype(pg_typeof(expression))

but there are other use-cases.  For example, if you want to know
the base types of the columns of a table, you could do something
like

select attname, pg_basetype(atttypid) from pg_attribute
  where attrelid = 'foo'::regclass order by attnum;

but that functionality is simply not available with the other
definition.

Perhaps there's an argument for providing both things, but that
feels like overkill to me.  I doubt that pg_basetype(pg_typeof())
is going to be so common as to need a shorthand.

            regards, tom lane



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
jian he
Дата:
On Mon, Mar 18, 2024 at 11:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Alexander Korotkov <aekorotkov@gmail.com> writes:
> > On Mon, Mar 18, 2024 at 2:01 AM jian he <jian.universality@gmail.com> wrote:
> >> `
> >> Datum
> >> pg_basetype(PG_FUNCTION_ARGS)
> >> {
> >>      Oid oid;
> >>
> >>      oid =  get_fn_expr_argtype(fcinfo->flinfo, 0);
> >>      PG_RETURN_OID(getBaseType(oid));
> >> }
> >> `
>
> > Looks good to me.  But should it be named pg_basetypeof()?
>
> I still don't like this approach.  It forces the function to be
> used in a particular way that's highly redundant with pg_typeof.
> I think we'd be better off with
>
> pg_basetype(PG_FUNCTION_ARGS)
> {
>         Oid typid = PG_GETARG_OID(0);
>
>         PG_RETURN_OID(getBaseType(typid));
> }
>
> The use-case that the other definition handles would be implemented
> like
>
>         pg_basetype(pg_typeof(expression))
>

trying to do it this way.
not sure the following error message is expected.

SELECT pg_basetype(-1);
ERROR:  cache lookup failed for type 4294967295

Вложения

Re: Add pg_basetype() function to obtain a DOMAIN base type

От
jian he
Дата:
On Thu, Mar 21, 2024 at 10:34 AM jian he <jian.universality@gmail.com> wrote:
>
> On Mon, Mar 18, 2024 at 11:43 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Alexander Korotkov <aekorotkov@gmail.com> writes:
> > > On Mon, Mar 18, 2024 at 2:01 AM jian he <jian.universality@gmail.com> wrote:
> > >> `
> > >> Datum
> > >> pg_basetype(PG_FUNCTION_ARGS)
> > >> {
> > >>      Oid oid;
> > >>
> > >>      oid =  get_fn_expr_argtype(fcinfo->flinfo, 0);
> > >>      PG_RETURN_OID(getBaseType(oid));
> > >> }
> > >> `
> >
> > > Looks good to me.  But should it be named pg_basetypeof()?
> >
> > I still don't like this approach.  It forces the function to be
> > used in a particular way that's highly redundant with pg_typeof.
> > I think we'd be better off with
> >
> > pg_basetype(PG_FUNCTION_ARGS)
> > {
> >         Oid typid = PG_GETARG_OID(0);
> >
> >         PG_RETURN_OID(getBaseType(typid));
> > }
> >
> > The use-case that the other definition handles would be implemented
> > like
> >
> >         pg_basetype(pg_typeof(expression))
> >
>
> trying to do it this way.
> not sure the following error message is expected.
>
> SELECT pg_basetype(-1);
> ERROR:  cache lookup failed for type 4294967295

I think the error message should be fine.
even though
`select '-1'::oid::regtype;` return 4294967295.

I noticed psql \dD didn't return the basetype of a domain.
one of the usage of this feature would be in psql \dD.

now we can:
\dD mytext_child_2
                                       List of domains
 Schema |      Name      |      Type      | Basetype | Collation |
Nullable | Default | Check
--------+----------------+----------------+----------+-----------+----------+---------+-------
 public | mytext_child_2 | mytext_child_1 | text     |           |
     |         |
(1 row)

Вложения

Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Tom Lane
Дата:
jian he <jian.universality@gmail.com> writes:
> I noticed psql \dD didn't return the basetype of a domain.
> one of the usage of this feature would be in psql \dD.

Your 0002 will cause \dD to fail entirely against an older server.
I'm not necessarily against adding this info, but you can't just
ignore the expectations for psql \d commands:

 * Support for the various \d ("describe") commands.  Note that the current
 * expectation is that all functions in this file will succeed when working
 * with servers of versions 9.2 and up.  It's okay to omit irrelevant
 * information for an old server, but not to fail outright.  (But failing
 * against a pre-9.2 server is allowed.)

            regards, tom lane



Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Tom Lane
Дата:
jian he <jian.universality@gmail.com> writes:
> trying to do it this way.
> not sure the following error message is expected.

> SELECT pg_basetype(-1);
> ERROR:  cache lookup failed for type 4294967295

Yeah, that's not really OK.  You could say it's fine for bogus input,
but we've found over the years that it's better for catalog inspection
functions like this to be forgiving of bad input.  Otherwise,
your query can blow up in unexpected ways due to race conditions
(ie somebody just dropped the type you are interested in).

A fairly common solution to that is to return NULL for bad input,
but in this case we could just have it return the OID unchanged.

Either way though, we can't use getBaseType as-is.  We could imagine
extending that function to support a "noerror"-like flag, but I
believe it's already a hot-spot and I'd rather not complicate it
further.  So what I suggest doing is just duplicating the code;
there's not very much of it.

I did a little polishing of the docs and test cases too, ending
with the v7 attached.  I think this is about ready to go unless
there are objections to the definition.

Not sure what I think about your 0002 proposal to extend \dD
with this.  Aside from the server-version-compatibility problem,
I think it's about 90% redundant because \dD already shows
the immediate base type.  The new column would only be
different in the case of nested domains, which I think are
not common.  \dD's output is already pretty wide, so on the
whole I'm inclined to leave it alone.

            regards, tom lane

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 93b0bc2bc6..b3687b3645 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -25129,6 +25129,29 @@ SELECT pg_type_is_visible('myschema.widget'::regtype);
        </para></entry>
       </row>

+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>pg_basetype</primary>
+        </indexterm>
+        <function>pg_basetype</function> ( <type>regtype</type> )
+        <returnvalue>regtype</returnvalue>
+       </para>
+       <para>
+        Returns the OID of the base type of a domain identified by its
+        type OID.  If the argument is not the OID of a domain type,
+        returns the argument as-is.  If there's a chain of domain
+        dependencies, it will recurse until finding the base type.
+       </para>
+       <para>
+        Assuming <literal>CREATE DOMAIN mytext AS text</literal>:
+       </para>
+       <para>
+        <literal>pg_basetype('mytext'::regtype)</literal>
+        <returnvalue>text</returnvalue>
+       </para></entry>
+      </row>
+
       <row>
        <entry id="pg-char-to-encoding" role="func_table_entry"><para role="func_signature">
         <indexterm>
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index d4a92d0b3f..d2b4ba8a72 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -44,6 +44,7 @@
 #include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/ruleutils.h"
+#include "utils/syscache.h"
 #include "utils/timestamp.h"


@@ -566,6 +567,48 @@ pg_typeof(PG_FUNCTION_ARGS)
 }


+/*
+ * Return the base type of the argument.
+ *        If the given type is a domain, return its base type;
+ *        otherwise return the type's own OID.
+ *
+ * This is a SQL-callable version of getBaseType().  Unlike that function,
+ * we don't want to fail for a bogus type OID; this is helpful to keep race
+ * conditions from turning into query failures when scanning the catalogs.
+ * Hence we need our own implementation.
+ */
+Datum
+pg_basetype(PG_FUNCTION_ARGS)
+{
+    Oid            typid = PG_GETARG_OID(0);
+
+    /*
+     * We loop to find the bottom base type in a stack of domains.
+     */
+    for (;;)
+    {
+        HeapTuple    tup;
+        Form_pg_type typTup;
+
+        tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typid));
+        if (!HeapTupleIsValid(tup))
+            break;                /* return the bogus OID as-is */
+        typTup = (Form_pg_type) GETSTRUCT(tup);
+        if (typTup->typtype != TYPTYPE_DOMAIN)
+        {
+            /* Not a domain, so done */
+            ReleaseSysCache(tup);
+            break;
+        }
+
+        typid = typTup->typbasetype;
+        ReleaseSysCache(tup);
+    }
+
+    PG_RETURN_OID(typid);
+}
+
+
 /*
  * Implementation of the COLLATE FOR expression; returns the collation
  * of the argument.
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 07023ee61d..134e3b22fd 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -3889,6 +3889,9 @@
 { oid => '1619', descr => 'type of the argument',
   proname => 'pg_typeof', proisstrict => 'f', provolatile => 's',
   prorettype => 'regtype', proargtypes => 'any', prosrc => 'pg_typeof' },
+{ oid => '8312', descr => 'base type of a domain type',
+  proname => 'pg_basetype', provolatile => 's', prorettype => 'regtype',
+  proargtypes => 'regtype', prosrc => 'pg_basetype' },
 { oid => '3162',
   descr => 'collation of the argument; implementation of the COLLATION FOR expression',
   proname => 'pg_collation_for', proisstrict => 'f', provolatile => 's',
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index dc58793e3f..71d9f1952c 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -1292,3 +1292,28 @@ SELECT * FROM information_schema.check_constraints
  regression         | public            | pos_int_not_null | VALUE IS NOT NULL
 (4 rows)

+--
+-- Get the base type of a domain
+--
+create domain mytext as text;
+create domain mytext_child_1 as mytext;
+select pg_basetype('mytext'::regtype);
+ pg_basetype
+-------------
+ text
+(1 row)
+
+select pg_basetype('mytext_child_1'::regtype);
+ pg_basetype
+-------------
+ text
+(1 row)
+
+select pg_basetype(1);  -- expect 1 not error
+ pg_basetype
+-------------
+ 1
+(1 row)
+
+drop domain mytext cascade;
+NOTICE:  drop cascades to type mytext_child_1
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index ae1b7fbf97..8a5121b056 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -862,3 +862,15 @@ SELECT * FROM information_schema.check_constraints
             FROM information_schema.domain_constraints
             WHERE domain_name IN ('con', 'dom', 'pos_int', 'things'))
   ORDER BY constraint_name;
+
+--
+-- Get the base type of a domain
+--
+create domain mytext as text;
+create domain mytext_child_1 as mytext;
+
+select pg_basetype('mytext'::regtype);
+select pg_basetype('mytext_child_1'::regtype);
+select pg_basetype(1);  -- expect 1 not error
+
+drop domain mytext cascade;

Re: Add pg_basetype() function to obtain a DOMAIN base type

От
Tom Lane
Дата:
I wrote:
> A fairly common solution to that is to return NULL for bad input,
> but in this case we could just have it return the OID unchanged.

After sleeping on it, I concluded that was a bad idea and we'd
be best off returning NULL for invalid type OIDs.  So this is
just about back to Steve's original proposal, except for being
a bit more bulletproof against races with DROP TYPE.
Pushed that way.

            regards, tom lane