diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2017-01-25 09:17:18 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2017-01-25 09:17:24 -0500 |
commit | 1e7c4bb0049732ece651d993d03bb6772e5d281a (patch) | |
tree | 801f99157b5ef0d582a3bfa3ba2a21507007fedf /src/test | |
parent | 123f03ba2c6e2d85a5a900e79dd5f216bfb37e25 (diff) |
Change unknown-type literals to type text in SELECT and RETURNING lists.
Previously, we left such literals alone if the query or subquery had
no properties forcing a type decision to be made (such as an ORDER BY or
DISTINCT clause using that output column). This meant that "unknown" could
be an exposed output column type, which has never been a great idea because
it could result in strange failures later on. For example, an outer query
that tried to do any operations on an unknown-type subquery output would
generally fail with some weird error like "failed to find conversion
function from unknown to text" or "could not determine which collation to
use for string comparison". Also, if the case occurred in a CREATE VIEW's
query then the view would have an unknown-type column, causing similar
failures in queries trying to use the view.
To fix, at the tail end of parse analysis of a query, forcibly convert any
remaining "unknown" literals in its SELECT or RETURNING list to type text.
However, provide a switch to suppress that, and use it in the cases of
SELECT inside a set operation or INSERT command. In those cases we already
had type resolution rules that make use of context information from outside
the subquery proper, and we don't want to change that behavior.
Also, change creation of an unknown-type column in a relation from a
warning to a hard error. The error should be unreachable now in CREATE
VIEW or CREATE MATVIEW, but it's still possible to explicitly say "unknown"
in CREATE TABLE or CREATE (composite) TYPE. We want to forbid that because
it's nothing but a foot-gun.
This change creates a pg_upgrade failure case: a matview that contains an
unknown-type column can't be pg_upgraded, because reparsing the matview's
defining query will now decide that the column is of type text, which
doesn't match the cstring-like storage that the old materialized column
would actually have. Add a checking pass to detect that. While at it,
we can detect tables or composite types that would fail, essentially
for free. Those would fail safely anyway later on, but we might as
well fail earlier.
This patch is by me, but it owes something to previous investigations
by Rahila Syed. Also thanks to Ashutosh Bapat and Michael Paquier for
review.
Discussion: https://postgr.es/m/CAH2L28uwwbL9HUM-WR=hromW1Cvamkn7O-g8fPY2m=_7muJ0oA@mail.gmail.com
Diffstat (limited to 'src/test')
-rw-r--r-- | src/test/regress/expected/create_table.out | 8 | ||||
-rw-r--r-- | src/test/regress/expected/create_view.out | 26 | ||||
-rw-r--r-- | src/test/regress/expected/matview.out | 27 | ||||
-rw-r--r-- | src/test/regress/expected/subselect.out | 31 | ||||
-rw-r--r-- | src/test/regress/expected/with.out | 20 | ||||
-rw-r--r-- | src/test/regress/output/create_function_1.source | 2 | ||||
-rw-r--r-- | src/test/regress/sql/create_table.sql | 8 | ||||
-rw-r--r-- | src/test/regress/sql/create_view.sql | 8 | ||||
-rw-r--r-- | src/test/regress/sql/matview.sql | 8 | ||||
-rw-r--r-- | src/test/regress/sql/subselect.sql | 10 | ||||
-rw-r--r-- | src/test/regress/sql/with.sql | 13 |
11 files changed, 154 insertions, 7 deletions
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 6caa9c2407d..36266f0a32b 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -199,6 +199,14 @@ CREATE TABLE array_index_op_test ( CREATE TABLE testjsonb ( j jsonb ); +CREATE TABLE unknowntab ( + u unknown -- fail +); +ERROR: column "u" has pseudo-type unknown +CREATE TYPE unknown_comptype AS ( + u unknown -- fail +); +ERROR: column "u" has pseudo-type unknown CREATE TABLE IF NOT EXISTS test_tsvector( t text, a tsvector diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out index 096bfc30c9e..ce0c8cedf89 100644 --- a/src/test/regress/expected/create_view.out +++ b/src/test/regress/expected/create_view.out @@ -288,6 +288,32 @@ SELECT relname, relkind, reloptions FROM pg_class mysecview4 | v | {security_barrier=false} (4 rows) +-- Check that unknown literals are converted to "text" in CREATE VIEW, +-- so that we don't end up with unknown-type columns. +CREATE VIEW unspecified_types AS + SELECT 42 as i, 42.5 as num, 'foo' as u, 'foo'::unknown as u2, null as n; +\d+ unspecified_types + View "testviewschm2.unspecified_types" + Column | Type | Collation | Nullable | Default | Storage | Description +--------+---------+-----------+----------+---------+----------+------------- + i | integer | | | | plain | + num | numeric | | | | main | + u | text | | | | extended | + u2 | text | | | | extended | + n | text | | | | extended | +View definition: + SELECT 42 AS i, + 42.5 AS num, + 'foo'::text AS u, + 'foo'::text AS u2, + NULL::text AS n; + +SELECT * FROM unspecified_types; + i | num | u | u2 | n +----+------+-----+-----+--- + 42 | 42.5 | foo | foo | +(1 row) + -- This test checks that proper typmods are assigned in a multi-row VALUES CREATE VIEW tt1 AS SELECT * FROM ( diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out index 7a2eaa0c4a8..4ae44607a40 100644 --- a/src/test/regress/expected/matview.out +++ b/src/test/regress/expected/matview.out @@ -508,6 +508,33 @@ DETAIL: drop cascades to materialized view mvtest_mv_v drop cascades to materialized view mvtest_mv_v_2 drop cascades to materialized view mvtest_mv_v_3 drop cascades to materialized view mvtest_mv_v_4 +-- Check that unknown literals are converted to "text" in CREATE MATVIEW, +-- so that we don't end up with unknown-type columns. +CREATE MATERIALIZED VIEW mv_unspecified_types AS + SELECT 42 as i, 42.5 as num, 'foo' as u, 'foo'::unknown as u2, null as n; +\d+ mv_unspecified_types + Materialized view "public.mv_unspecified_types" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+----------+--------------+------------- + i | integer | | | | plain | | + num | numeric | | | | main | | + u | text | | | | extended | | + u2 | text | | | | extended | | + n | text | | | | extended | | +View definition: + SELECT 42 AS i, + 42.5 AS num, + 'foo'::text AS u, + 'foo'::text AS u2, + NULL::text AS n; + +SELECT * FROM mv_unspecified_types; + i | num | u | u2 | n +----+------+-----+-----+--- + 42 | 42.5 | foo | foo | +(1 row) + +DROP MATERIALIZED VIEW mv_unspecified_types; -- make sure that create WITH NO DATA does not plan the query (bug #13907) create materialized view mvtest_error as select 1/0 as x; -- fail ERROR: division by zero diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index abd3217e866..47afdc335e1 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -196,6 +196,37 @@ SELECT '' AS five, f1 AS "Correlated Field" | 3 (5 rows) +-- Unspecified-type literals in output columns should resolve as text +SELECT *, pg_typeof(f1) FROM + (SELECT 'foo' AS f1 FROM generate_series(1,3)) ss ORDER BY 1; + f1 | pg_typeof +-----+----------- + foo | text + foo | text + foo | text +(3 rows) + +-- ... unless there's context to suggest differently +explain verbose select '42' union all select '43'; + QUERY PLAN +------------------------------------------------- + Append (cost=0.00..0.04 rows=2 width=32) + -> Result (cost=0.00..0.01 rows=1 width=32) + Output: '42'::text + -> Result (cost=0.00..0.01 rows=1 width=32) + Output: '43'::text +(5 rows) + +explain verbose select '42' union all select 43; + QUERY PLAN +------------------------------------------------ + Append (cost=0.00..0.04 rows=2 width=4) + -> Result (cost=0.00..0.01 rows=1 width=4) + Output: 42 + -> Result (cost=0.00..0.01 rows=1 width=4) + Output: 43 +(5 rows) + -- -- Use some existing tables in the regression test -- diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out index 02fa08e932f..3b7f689a98b 100644 --- a/src/test/regress/expected/with.out +++ b/src/test/regress/expected/with.out @@ -133,9 +133,9 @@ SELECT * FROM t LIMIT 10; -- Test behavior with an unknown-type literal in the WITH WITH q AS (SELECT 'foo' AS x) -SELECT x, x IS OF (unknown) as is_unknown FROM q; - x | is_unknown ------+------------ +SELECT x, x IS OF (text) AS is_text FROM q; + x | is_text +-----+--------- foo | t (1 row) @@ -144,7 +144,7 @@ WITH RECURSIVE t(n) AS ( UNION ALL SELECT n || ' bar' FROM t WHERE length(n) < 20 ) -SELECT n, n IS OF (text) as is_text FROM t; +SELECT n, n IS OF (text) AS is_text FROM t; n | is_text -------------------------+--------- foo | t @@ -155,6 +155,18 @@ SELECT n, n IS OF (text) as is_text FROM t; foo bar bar bar bar bar | t (6 rows) +-- In a perfect world, this would work and resolve the literal as int ... +-- but for now, we have to be content with resolving to text too soon. +WITH RECURSIVE t(n) AS ( + SELECT '7' +UNION ALL + SELECT n+1 FROM t WHERE n < 10 +) +SELECT n, n IS OF (int) AS is_int FROM t; +ERROR: operator does not exist: text + integer +LINE 4: SELECT n+1 FROM t WHERE n < 10 + ^ +HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. -- -- Some examples with a tree -- diff --git a/src/test/regress/output/create_function_1.source b/src/test/regress/output/create_function_1.source index 30c2936f8d1..957595c51e4 100644 --- a/src/test/regress/output/create_function_1.source +++ b/src/test/regress/output/create_function_1.source @@ -59,7 +59,7 @@ CREATE FUNCTION test_atomic_ops() CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL AS 'SELECT ''not an integer'';'; ERROR: return type mismatch in function declared to return integer -DETAIL: Actual return type is unknown. +DETAIL: Actual return type is text. CONTEXT: SQL function "test1" CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL AS 'not even SQL'; diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index 8242e7328d8..6314aa403ff 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -236,6 +236,14 @@ CREATE TABLE testjsonb ( j jsonb ); +CREATE TABLE unknowntab ( + u unknown -- fail +); + +CREATE TYPE unknown_comptype AS ( + u unknown -- fail +); + CREATE TABLE IF NOT EXISTS test_tsvector( t text, a tsvector diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql index 5fe8b94aae0..c27f1034e13 100644 --- a/src/test/regress/sql/create_view.sql +++ b/src/test/regress/sql/create_view.sql @@ -224,6 +224,14 @@ SELECT relname, relkind, reloptions FROM pg_class 'mysecview3'::regclass, 'mysecview4'::regclass) ORDER BY relname; +-- Check that unknown literals are converted to "text" in CREATE VIEW, +-- so that we don't end up with unknown-type columns. + +CREATE VIEW unspecified_types AS + SELECT 42 as i, 42.5 as num, 'foo' as u, 'foo'::unknown as u2, null as n; +\d+ unspecified_types +SELECT * FROM unspecified_types; + -- This test checks that proper typmods are assigned in a multi-row VALUES CREATE VIEW tt1 AS diff --git a/src/test/regress/sql/matview.sql b/src/test/regress/sql/matview.sql index 65a743ced96..1164b4cea21 100644 --- a/src/test/regress/sql/matview.sql +++ b/src/test/regress/sql/matview.sql @@ -198,6 +198,14 @@ SELECT * FROM mvtest_mv_v_3; SELECT * FROM mvtest_mv_v_4; DROP TABLE mvtest_v CASCADE; +-- Check that unknown literals are converted to "text" in CREATE MATVIEW, +-- so that we don't end up with unknown-type columns. +CREATE MATERIALIZED VIEW mv_unspecified_types AS + SELECT 42 as i, 42.5 as num, 'foo' as u, 'foo'::unknown as u2, null as n; +\d+ mv_unspecified_types +SELECT * FROM mv_unspecified_types; +DROP MATERIALIZED VIEW mv_unspecified_types; + -- make sure that create WITH NO DATA does not plan the query (bug #13907) create materialized view mvtest_error as select 1/0 as x; -- fail create materialized view mvtest_error as select 1/0 as x with no data; diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index 08eb825c542..9c2a73d4d77 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -80,6 +80,16 @@ SELECT '' AS five, f1 AS "Correlated Field" WHERE (f1, f2) IN (SELECT f2, CAST(f3 AS int4) FROM SUBSELECT_TBL WHERE f3 IS NOT NULL); +-- Unspecified-type literals in output columns should resolve as text + +SELECT *, pg_typeof(f1) FROM + (SELECT 'foo' AS f1 FROM generate_series(1,3)) ss ORDER BY 1; + +-- ... unless there's context to suggest differently + +explain verbose select '42' union all select '43'; +explain verbose select '42' union all select 43; + -- -- Use some existing tables in the regression test -- diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql index 7ee32bab8f6..08ddc8bae01 100644 --- a/src/test/regress/sql/with.sql +++ b/src/test/regress/sql/with.sql @@ -69,14 +69,23 @@ SELECT * FROM t LIMIT 10; -- Test behavior with an unknown-type literal in the WITH WITH q AS (SELECT 'foo' AS x) -SELECT x, x IS OF (unknown) as is_unknown FROM q; +SELECT x, x IS OF (text) AS is_text FROM q; WITH RECURSIVE t(n) AS ( SELECT 'foo' UNION ALL SELECT n || ' bar' FROM t WHERE length(n) < 20 ) -SELECT n, n IS OF (text) as is_text FROM t; +SELECT n, n IS OF (text) AS is_text FROM t; + +-- In a perfect world, this would work and resolve the literal as int ... +-- but for now, we have to be content with resolving to text too soon. +WITH RECURSIVE t(n) AS ( + SELECT '7' +UNION ALL + SELECT n+1 FROM t WHERE n < 10 +) +SELECT n, n IS OF (int) AS is_int FROM t; -- -- Some examples with a tree |