Skip to content

Commit

Permalink
Replace C parse_type() function with PL/pgSQL
Browse files Browse the repository at this point in the history
The C code cause a number of challenges, including the requirement for a
superuser to install the extension, and would require a fair bit of
refactoring to abide by [secure design principals]. It also makes
installation difficult on Windows, and would likely be rejected by
organizations like AWS that tend to balk at C code.

So delete the `parse_type()` function and all the C code added in
4ec32e7 and rewrite the `format_type_string()` function based on the
prototype that @ewie developed in the comments of #315. By looking up
the type output format function in the catalog, we're able to correctly
output the data type with its full SQL name as before. The exception is
interval types, which require the PostgreSQL grammar to parse the
interval fields (`second`, `interval minute`, etc.) into a bitmask for
the `typmod`. So require that they be specified exactly as PostgreSQL
outputs them.

  [secure design principals]: https://www.postgresql.org/docs/current/extend-extensions.html#EXTEND-EXTENSIONS-SECURITY
  • Loading branch information
theory committed Feb 3, 2024
1 parent c47f862 commit 8fab510
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 125 deletions.
8 changes: 8 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ Revision history for pgTAP
1.3.2
--------------------------

* Replaced the C `parse_type()` function added in v1.3.1 with a PL/pgSQL
function, eliminating the need for the the installing use to be a superuser.
Aliases still work for all data types passed to `col_type_is()` except for
intervals, which must be specified exactly as rendered by Postgres itself,
without aliasing. Thanks to @spencerbryson for the report (#328) and to
Erik Wienhold for the PL/pgSQL technique necessary to properly format
type strings (#315).

1.3.1 2023-09-24T15:29:42Z
--------------------------
* Revamped the handling of data type declarations in `col_type_is()` to always
Expand Down
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ EXTRA_CLEAN = $(VERSION_FILES) sql/pgtap.sql sql/uninstall_pgtap.sql sql/pgtap-
EXTRA_CLEAN += $(wildcard sql/*.orig) # These are files left behind by patch
DOCS = doc/pgtap.mmd
PG_CONFIG ?= pg_config
MODULES = src/pgtap

#
# Test configuration. This must be done BEFORE including PGXS
Expand Down
39 changes: 11 additions & 28 deletions doc/pgtap.mmd
Original file line number Diff line number Diff line change
Expand Up @@ -4524,6 +4524,11 @@ pass the type as either "varchar(64)" or "character varying(64)". Example:

SELECT col_type_is( 'myschema', 'sometable', 'somecolumn', 'timespantz(3)' );

The exception to this rule is interval types, which must be specified as
rendered by PostgreSQL itself:

SELECT col_type_is( 'myschema', 'sometable', 'somecolumn', 'interval second(3)' );

Types with case-sensitive names or special characters must be double-quoted:

SELECT col_type_is( 'myschema', 'sometable', 'somecolumn', '"myType"' );
Expand Down Expand Up @@ -8293,31 +8298,6 @@ included in the display. For example:
Used internally by pgTAP to compare operators, but may be more generally
useful.

### `parse_type()` ###

SELECT * FROM parse_type( :text );

**Parameters**

`:text`
: An SQL type declaration, optionally schema-qualified.

Parses a string representing an SQL type declaration as used in a `CREATE TABLE`
statement, optionally schema-qualified. Returns a record with two fields,
`typid` and `typmod`, representing the OID and modifier for the type. These are
the underlying values that define column data types, and which can be passed to
the PostgreSQL core
[`format_type()`](https://www.postgresql.org/docs/current/functions-info.html#FUNCTIONS-INFO-CATALOG)
function to display the normalized string representation of the data type.
Raises an error if the specify type does not exist or cannot be found in the
search path.

try=% SELECT format_type(p.typid, p.typmod)
try-% FROM parse_type('timestamp(4)') p;
format_type
--------------------------------
timestamp(4) without time zone

### `format_type_string()` ###

SELECT format_type_string( :text );
Expand All @@ -8329,9 +8309,12 @@ search path.

This function normalizes data type declarations for accurate comparison
to table columns by `col_type_is()`. It's effectively the identical to
the calling `format_type()` with the values returned by `parse_type()`,
but returns a `NULL` on an invalid or missing type, rather than raising
an error.
the calling `format_type()` with the type OID and type modifier that define
the column, but returns a `NULL` on an invalid or missing type, rather than
raising an error. Types can be defined by their canonical names or their
aliases, e.g., `character varying` or `varchar`. The exception is `interval`
types, which must be specified exactly as Postgres renders them internally,
e.g., `'interval(0)`, `interval second(0)`, or `interval day to second(4)`.

try=# SELECT format_type_string('timestamp(3)');
format_type_string
Expand Down
34 changes: 34 additions & 0 deletions sql/pgtap--1.3.1--1.3.2.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
DROP FUNCTION parse_type(type text, OUT typid oid, OUT typmod int4);

CREATE OR REPLACE FUNCTION format_type_string ( TEXT )
RETURNS TEXT AS $$
DECLARE
want_type TEXT := $1;
typmodin_arg cstring[];
typmodin_func regproc;
typmod int;
BEGIN
IF want_type::regtype = 'interval'::regtype THEN
-- RAISE NOTICE 'cannot resolve: %', want_type; -- TODO
RETURN want_type;
END IF;

-- Extract type modifier from type declaration and format as cstring[] literal.
typmodin_arg := translate(substring(want_type FROM '[(][^")]+[)]'), '()', '{}');

-- Find typmodin function for want_type.
SELECT typmodin INTO typmodin_func
FROM pg_catalog.pg_type
WHERE oid = want_type::regtype;

IF typmodin_func = 0 THEN
-- Easy: types without typemods.
RETURN format_type(want_type::regtype, null);
END IF;

-- Get typemod via type-specific typmodin function.
EXECUTE format('SELECT %I(%L)', typmodin_func, typmodin_arg) INTO typmod;
RETURN format_type(want_type::regtype, typmod);
EXCEPTION WHEN OTHERS THEN RETURN NULL;
END;
$$ LANGUAGE PLPGSQL STABLE;
33 changes: 27 additions & 6 deletions sql/pgtap.sql.in
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ CREATE OR REPLACE FUNCTION pgtap_version()
RETURNS NUMERIC AS 'SELECT __VERSION__;'
LANGUAGE SQL IMMUTABLE;

CREATE FUNCTION parse_type(type text, OUT typid oid, OUT typmod int4)
RETURNS RECORD
AS '$libdir/pgtap'
LANGUAGE C STABLE STRICT;

CREATE OR REPLACE FUNCTION plan( integer )
RETURNS TEXT AS $$
DECLARE
Expand Down Expand Up @@ -1468,7 +1463,33 @@ $$ LANGUAGE PLPGSQL STABLE;

CREATE OR REPLACE FUNCTION format_type_string ( TEXT )
RETURNS TEXT AS $$
BEGIN RETURN format_type(p.typid, p.typmod) from parse_type($1) p;
DECLARE
want_type TEXT := $1;
typmodin_arg cstring[];
typmodin_func regproc;
typmod int;
BEGIN
IF want_type::regtype = 'interval'::regtype THEN
-- RAISE NOTICE 'cannot resolve: %', want_type; -- TODO
RETURN want_type;
END IF;

-- Extract type modifier from type declaration and format as cstring[] literal.
typmodin_arg := translate(substring(want_type FROM '[(][^")]+[)]'), '()', '{}');

-- Find typmodin function for want_type.
SELECT typmodin INTO typmodin_func
FROM pg_catalog.pg_type
WHERE oid = want_type::regtype;

IF typmodin_func = 0 THEN
-- Easy: types without typemods.
RETURN format_type(want_type::regtype, null);
END IF;

-- Get typemod via type-specific typmodin function.
EXECUTE format('SELECT %I(%L)', typmodin_func, typmodin_arg) INTO typmod;
RETURN format_type(want_type::regtype, typmod);
EXCEPTION WHEN OTHERS THEN RETURN NULL;
END;
$$ LANGUAGE PLPGSQL STABLE;
Expand Down
90 changes: 0 additions & 90 deletions src/pgtap.c

This file was deleted.

0 comments on commit 8fab510

Please sign in to comment.