Обсуждение: Add pg_basetype() function to obtain a DOMAIN base type
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;
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;
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
----------------+------
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)
pg_basetype
-------------
text
(1 row)
```
Tests and docs are added.
Best regards,
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
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
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
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;
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