Hi David, Thanks for the patch. On 04.02.24 18:51, David E. Wheeler wrote: > Hackers, > > Attached is a patch to add a new function, `parse_type()`. It parses a type > string and returns a record with the typid and typmod for the type, or raises > an error if the type is invalid. It’s effectively a thin layer over the > parser’s parseTypeString() function. > > The purpose of this function is to allow uses to resolve any type name, > including aliases, to its formal name as rendered by, for example, pg_dump. > An example: > > david=# WITH s(s) AS ( > SELECT * FROM unnest(ARRAY[ > 'timestamp(4)', > 'interval(0)', > 'interval second(0)', > 'timestamptz', > 'timestamptz(6)', > 'varchar', > 'varchar(128)' > ]) > ), > p(type, typid, typmod) AS ( > SELECT s, ((parse_type(s)).*) > FROM s > ) > SELECT type, format_type(typid, typmod) FROM p; > type | format_type > --------------------+-------------------------------- > timestamp(4) | timestamp(4) without time zone > interval(0) | interval(0) > interval second(0) | interval second(0) > timestamptz | timestamp with time zone > timestamptz(6) | timestamp(6) with time zone > varchar | character varying > varchar(128) | character varying(128) > (7 rows) > > > The use case leading to this patch was pgTAP column data type assertions. > pgTAP traditionally required full type names for testing data types, but > recently added support for aliases. This broke for some types, because it was > difficult to extract the typmod correctly, and even when we did, it failed > for types such as `interval second(0)`, where `second (0)` is the typmod, and > there was no sensible way to access the bit mask to generate the proper > typmod to pass to `format_type()`. > > Erik Wienhold wrote this function to solve the problem, and I volunteered to > submit a patch. > > Potential issues and questions for this patch: > > * Is `parse_type()` the right name to use? I can see arguments for > `parse_type_string()`, `pg_parse_type()`, and `pg_parse_type_string()`. > Whatever the consensus is works for me. > > * The function returns a record, which is a little fussy. I could see adding > a `format_type_string()` function that just contains `SELECT > format_type(p.typmod, p.typid) FROM parse_type($1) p` and returns the > formatted type. But I think it might also be useful to have access to the > typmod and typid directly via this method, since they’re already exposed as > `format_type()`’s arguments. > > * Is format_type.c the right home for the function? I put it there because I > closely associate it with format_type(), but happy to move it to a more > appropriate location. > > * I tried to link to `format_type` from the docs entry for `parse_type`, and > added an ID for the former, but I’m not sure it resolves. > > Best, > > David > The patch applies cleanly
Checking patch doc/src/sgml/func.sgml... Checking patch src/backend/utils/adt/format_type.c... Checking patch src/include/catalog/pg_proc.dat... Checking patch src/include/utils/builtins.h... Checking patch src/test/regress/expected/create_type.out... Checking patch src/test/regress/sql/create_type.sql... Applied patch doc/src/sgml/func.sgml cleanly. Applied patch src/backend/utils/adt/format_type.c cleanly. Applied patch src/include/catalog/pg_proc.dat cleanly. Applied patch src/include/utils/builtins.h cleanly. Applied patch src/test/regress/expected/create_type.out cleanly. Applied patch src/test/regress/sql/create_type.sql cleanly. The docs render just fine and all tests pass (check and check-world). There are a few issues though: 1) The function causes a crash when called with a NULL parameter: SELECT * FROM parse_type(NULL); server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. The connection to the server was lost. Attempting reset: Failed. You have to check it in the beginning of function and either return an error message or just NULL, e.g if (PG_ARGISNULL(0)) PG_RETURN_NULL(); 2) Add a function call with NULL in the regression tests SELECT * FROM parse_type(NULL); 3) Add the expected behaviour of calling the function with NULL in the docs (error message or null) 4) The name of the returned columns is repeated in the docs > Returns a record with two fields, <parameter>typid</parameter> and <parameter>typid</parameter> -- Jim
