Add pg_basetype() function to obtain a DOMAIN base type

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, like postgREST 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.

Best regards,
Steve Chavez

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

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, like postgREST 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

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.

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?

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.

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

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.

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

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

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

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

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.

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.

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.

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

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

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

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)

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