Skip to content

Commit

Permalink
Improve data type parsing and formatting
Browse files Browse the repository at this point in the history
Add a C module to delegate the parsing of data types by `col_type_is()`
to the Postgres core `parseTypeString()` function, which is the
canonical type parser. This ensure that no matter the aliasing or
typemod specified for a data type, we end up with exactly the same
spelling as the core uses, ensuring more accurate comparisons. This was
necessitated by the change in 1e1d745 that added support for type
aliases but broke complicated typmods such as ` second(0)` in `interval
second(0)`. Resolves #315.

Document the two new functions, `parse_type()` and
`format_type_string()` and remove the documentation for `pg_typeof()`,
which hasn't shipped with pgTAP in several years, since it has been in
the Postgres core since 8.3.

Also, fix the name of the v1.3.9 upgrade file.
  • Loading branch information
theory committed Sep 18, 2023
1 parent 8a73cbe commit 56e1f40
Show file tree
Hide file tree
Showing 10 changed files with 616 additions and 225 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ pgtap-schema.sql
uninstall_pgtap.sql
results
pgtap.so
pgtap.dylib
pgtap.dll
regression.*
*.html
*.html1
Expand Down
12 changes: 12 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,18 @@ Revision history for pgTAP
1.3.1
--------------------------

* Revamped the handling of data type declarations in `col_type_is()` to always
accurately normalize data type representations, whether using aliases, such
as `varchar` for `character varying`, or more complicated types that failed
to work in v1.3.0, such as `interval second(0)`. This is done by delegating
the parsing of the type declaration to a core PostgresSQL function. Many
thanks to Erik Wienhold for the report and for ultimately uncovering and
implementing an SQL interface to the `parseTypeString()` core function
(#315).
* Removed the documentation for `pg_typeof()`, which was removed from pgTAP
when support for PostgreSQL 8.3 was dropped, since the same function has
been available in the PostgreSQL core since then.

1.3.0 2023-08-14T22:14:20Z
--------------------------
* Fixed an issue with xUnit tests where they would exit immediately on
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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
80 changes: 54 additions & 26 deletions doc/pgtap.mmd
Original file line number Diff line number Diff line change
Expand Up @@ -375,9 +375,9 @@ discrepancy between the planned number of tests and the number actually run:

If you need to throw an exception if some test failed, you can pass an
option to `finish()`.
SELECT * FROM finish(true);

SELECT * FROM finish(true);

What a sweet unit!
------------------

Expand Down Expand Up @@ -4504,22 +4504,24 @@ fourth the type's schema, the fifth the type, and the sixth is the test
description.

If the table schema is omitted, the table must be visible in the search path.
If the type schema is omitted, it must be visible in the search path;
otherwise, the diagnostics will report the schema it's actually in. The schema
can optionally be included in the `:type` argument, e.g., `'contrib.citext`.
If the type schema is omitted, it must be visible in the search path. The
schema can optionally be included in the `:type` argument, e.g.,
`'contrib.citext`.

If the test description is omitted, it will be set to "Column
`:schema.:table.:column` should be type `:schema.:type`". Note that this test
will fail if the table or column in question does not exist.

The type argument should be formatted as it would be displayed in the view of
a table using the `\d` command in `psql`. For example, if you have a numeric
column with a precision of 8, you should specify "numeric(8,0)". If you
created a `varchar(64)` column, you should pass the type as "varchar(64)" or
"character varying(64)". Example:
The type argument should may be formatted using the full name of the type or
any supported alias. For example, if you created a `varchar(64)` column, you
can pass the type as either "varchar(64)" or "character varying(64)". Example:

SELECT col_type_is( 'myschema', 'sometable', 'somecolumn', 'numeric(10,2)' );

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

SELECT col_type_is( 'myschema', 'sometable', 'somecolumn', '"myType"' );

If the test fails, it will output useful diagnostics. For example this test:

SELECT col_type_is( 'pg_catalog', 'pg_type', 'typname', 'text' );
Expand All @@ -4530,8 +4532,8 @@ Will produce something like this:
# have: name
# want: text

It will even tell you if the test fails because a column doesn't exist or
actually has no default. But use `has_column()` to make sure the column exists
It will even tell you if the test fails because a column doesn't exist or if
the type doesn't exist. But use `has_column()` to make sure the column exists
first, eh?

### `col_default_is()` ###
Expand Down Expand Up @@ -8190,7 +8192,7 @@ the stringified version number displayed in the first part of the core
`version()` function and stored in the "server_version" setting:

try=% select current_setting( 'server_version'), pg_version();
current_setting | pg_version
current_setting | pg_version
-----------------+------------
12.2 | 12.2
(1 row)
Expand Down Expand Up @@ -8273,23 +8275,50 @@ included in the display. For example:
Used internally by pgTAP to compare operators, but may be more generally
useful.

### `pg_typeof()` ###
### `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 type OID and type 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.

SELECT pg_typeof(:any);
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 );

**Parameters**

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

Returns a `regtype` identifying the type of value passed to the function. This
function is used internally by `cmp_ok()` to properly construct types when
executing the comparison, but might be generally useful.
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.

try=% select pg_typeof(12), pg_typeof(100.2);
pg_typeof | pg_typeof
-----------+-----------
integer | numeric
try=# SELECT format_type_string('timestamp(3)');
format_type_string
--------------------------------
timestamp(3) without time zone

### `findfuncs()` ###

Expand All @@ -8309,7 +8338,6 @@ executing the comparison, but might be generally useful.
`:pattern`
: Regular expression pattern to exclude functions with matching names.


This function searches the named schema or, if no schema is passed, the search
patch, for all functions that match the regular expression pattern. The
optional exclude regular expression pattern can be used to prevent matchin
Expand Down
File renamed without changes.
98 changes: 98 additions & 0 deletions sql/pgtap--1.3.0--1.3.1.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
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 format_type_string ( TEXT )
RETURNS TEXT AS $$
BEGIN RETURN format_type(p.typid, p.typmod) from parse_type($1) p;
EXCEPTION WHEN OTHERS THEN RETURN NULL;
END;
$$ LANGUAGE PLPGSQL STABLE;

-- col_type_is( schema, table, column, schema, type, description )
CREATE OR REPLACE FUNCTION col_type_is ( NAME, NAME, NAME, NAME, TEXT, TEXT )
RETURNS TEXT AS $$
DECLARE
have_type TEXT := _get_col_ns_type($1, $2, $3);
want_type TEXT;
BEGIN
IF have_type IS NULL THEN
RETURN fail( $6 ) || E'\n' || diag (
' Column ' || COALESCE(quote_ident($1) || '.', '')
|| quote_ident($2) || '.' || quote_ident($3) || ' does not exist'
);
END IF;

IF quote_ident($4) = ANY(current_schemas(true)) THEN
want_type := quote_ident($4) || '.' || format_type_string($5);
ELSE
want_type := format_type_string(quote_ident($4) || '.' || $5);
END IF;

IF want_type IS NULL THEN
RETURN fail( $6 ) || E'\n' || diag (
' Type ' || quote_ident($4) || '.' || $5 || ' does not exist'
);
END IF;

IF have_type = want_type THEN
-- We're good to go.
RETURN ok( true, $6 );
END IF;

-- Wrong data type. tell 'em what we really got.
RETURN ok( false, $6 ) || E'\n' || diag(
' have: ' || have_type ||
E'\n want: ' || want_type
);
END;
$$ LANGUAGE plpgsql;

-- col_type_is( schema, table, column, schema, type )
CREATE OR REPLACE FUNCTION col_type_is ( NAME, NAME, NAME, NAME, TEXT )
RETURNS TEXT AS $$
SELECT col_type_is( $1, $2, $3, $4, $5, 'Column ' || quote_ident($1) || '.' || quote_ident($2)
|| '.' || quote_ident($3) || ' should be type ' || quote_ident($4) || '.' || $5);
$$ LANGUAGE SQL;

-- col_type_is( schema, table, column, type, description )
CREATE OR REPLACE FUNCTION col_type_is ( NAME, NAME, NAME, TEXT, TEXT )
RETURNS TEXT AS $$
DECLARE
have_type TEXT;
want_type TEXT;
BEGIN
-- Get the data type.
IF $1 IS NULL THEN
have_type := _get_col_type($2, $3);
ELSE
have_type := _get_col_type($1, $2, $3);
END IF;

IF have_type IS NULL THEN
RETURN fail( $5 ) || E'\n' || diag (
' Column ' || COALESCE(quote_ident($1) || '.', '')
|| quote_ident($2) || '.' || quote_ident($3) || ' does not exist'
);
END IF;

want_type := format_type_string($4);
IF want_type IS NULL THEN
RETURN fail( $5 ) || E'\n' || diag (
' Type ' || $4 || ' does not exist'
);
END IF;

IF have_type = want_type THEN
-- We're good to go.
RETURN ok( true, $5 );
END IF;

-- Wrong data type. tell 'em what we really got.
RETURN ok( false, $5 ) || E'\n' || diag(
' have: ' || have_type ||
E'\n want: ' || want_type
);
END;
$$ LANGUAGE plpgsql;
33 changes: 31 additions & 2 deletions sql/pgtap.sql.in
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ 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 @@ -1461,6 +1466,13 @@ EXCEPTION WHEN undefined_object THEN RETURN $1;
END;
$$ 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;
EXCEPTION WHEN OTHERS THEN RETURN NULL;
END;
$$ LANGUAGE PLPGSQL STABLE;

CREATE OR REPLACE FUNCTION _quote_ident_like(TEXT, TEXT)
RETURNS TEXT AS $$
DECLARE
Expand Down Expand Up @@ -1489,7 +1501,18 @@ BEGIN
);
END IF;

want_type := quote_ident($4) || '.' || _quote_ident_like($5, have_type);
IF quote_ident($4) = ANY(current_schemas(true)) THEN
want_type := quote_ident($4) || '.' || format_type_string($5);
ELSE
want_type := format_type_string(quote_ident($4) || '.' || $5);
END IF;

IF want_type IS NULL THEN
RETURN fail( $6 ) || E'\n' || diag (
' Type ' || quote_ident($4) || '.' || $5 || ' does not exist'
);
END IF;

IF have_type = want_type THEN
-- We're good to go.
RETURN ok( true, $6 );
Expand Down Expand Up @@ -1531,7 +1554,13 @@ BEGIN
);
END IF;

want_type := _quote_ident_like($4, have_type);
want_type := format_type_string($4);
IF want_type IS NULL THEN
RETURN fail( $5 ) || E'\n' || diag (
' Type ' || $4 || ' does not exist'
);
END IF;

IF have_type = want_type THEN
-- We're good to go.
RETURN ok( true, $5 );
Expand Down
Loading

0 comments on commit 56e1f40

Please sign in to comment.