aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2016-04-21 14:20:18 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2016-04-21 14:20:18 -0400
commit663624e60bd679ef7d4abeca1e68aceb5f2f5bbe (patch)
treefedf798bde37c6a3db9bc674606752c9e875901e
parent4c1c9f80b769120f2e809aa6447383c95c7705cf (diff)
downloadpostgresql-663624e60bd679ef7d4abeca1e68aceb5f2f5bbe.tar.gz
postgresql-663624e60bd679ef7d4abeca1e68aceb5f2f5bbe.zip
Fix ruleutils.c's dumping of ScalarArrayOpExpr containing an EXPR_SUBLINK.
When we shoehorned "x op ANY (array)" into the SQL syntax, we created a fundamental ambiguity as to the proper treatment of a sub-SELECT on the righthand side: perhaps what's meant is to compare x against each row of the sub-SELECT's result, or perhaps the sub-SELECT is meant as a scalar sub-SELECT that delivers a single array value whose members should be compared against x. The grammar resolves it as the former case whenever the RHS is a select_with_parens, making the latter case hard to reach --- but you can get at it, with tricks such as attaching a no-op cast to the sub-SELECT. Parse analysis would throw away the no-op cast, leaving a parsetree with an EXPR_SUBLINK SubLink directly under a ScalarArrayOpExpr. ruleutils.c was not clued in on this fine point, and would naively emit "x op ANY ((SELECT ...))", which would be parsed as the first alternative, typically leading to errors like "operator does not exist: text = text[]" during dump/reload of a view or rule containing such a construct. To fix, emit a no-op cast when dumping such a parsetree. This might well be exactly what the user wrote to get the construct accepted in the first place; and even if she got there with some other dodge, it is a valid representation of the parsetree. Per report from Karl Czajkowski. He mentioned only a case involving RLS policies, but actually the problem is very old, so back-patch to all supported branches. Report: <20160421001832.GB7976@moraine.isi.edu>
-rw-r--r--src/backend/utils/adt/ruleutils.c18
-rw-r--r--src/test/regress/expected/create_view.out27
-rw-r--r--src/test/regress/sql/create_view.sql11
3 files changed, 56 insertions, 0 deletions
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index ddd1c863ccd..8af9b9a4a9d 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -5053,6 +5053,24 @@ get_rule_expr(Node *node, deparse_context *context,
get_base_element_type(exprType(arg2))),
expr->useOr ? "ANY" : "ALL");
get_rule_expr_paren(arg2, context, true, node);
+
+ /*
+ * There's inherent ambiguity in "x op ANY/ALL (y)" when y is
+ * a bare sub-SELECT. Since we're here, the sub-SELECT must
+ * be meant as a scalar sub-SELECT yielding an array value to
+ * be used in ScalarArrayOpExpr; but the grammar will
+ * preferentially interpret such a construct as an ANY/ALL
+ * SubLink. To prevent misparsing the output that way, insert
+ * a dummy coercion (which will be stripped by parse analysis,
+ * so no inefficiency is added in dump and reload). This is
+ * indeed most likely what the user wrote to get the construct
+ * accepted in the first place.
+ */
+ if (IsA(arg2, SubLink) &&
+ ((SubLink *) arg2)->subLinkType == EXPR_SUBLINK)
+ appendStringInfo(buf, "::%s",
+ format_type_with_typemod(exprType(arg2),
+ exprTypmod(arg2)));
appendStringInfoChar(buf, ')');
if (!PRETTY_PAREN(context))
appendStringInfoChar(buf, ')');
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 56a1566aeed..3dfd44de246 100644
--- a/src/test/regress/expected/create_view.out
+++ b/src/test/regress/expected/create_view.out
@@ -298,6 +298,33 @@ select * from int8_tbl i where i.* in (values(i.*::int8_tbl));
4567890123456789 | -4567890123456789
(5 rows)
+-- check display of ScalarArrayOp with a sub-select
+select 'foo'::text = any(array['abc','def','foo']::text[]);
+ ?column?
+----------
+ t
+(1 row)
+
+select 'foo'::text = any((select array['abc','def','foo']::text[])); -- fail
+ERROR: operator does not exist: text = text[]
+LINE 1: select 'foo'::text = any((select array['abc','def','foo']::t...
+ ^
+HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
+select 'foo'::text = any((select array['abc','def','foo']::text[])::text[]);
+ ?column?
+----------
+ t
+(1 row)
+
+create view tt19v as
+select 'foo'::text = any(array['abc','def','foo']::text[]) c1,
+ 'foo'::text = any((select array['abc','def','foo']::text[])::text[]) c2;
+select pg_get_viewdef('tt19v', true);
+ pg_get_viewdef
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ SELECT 'foo'::text = ANY (ARRAY['abc'::text, 'def'::text, 'foo'::text]) AS c1, 'foo'::text = ANY ((( SELECT ARRAY['abc'::text, 'def'::text, 'foo'::text] AS "array"))::text[]) AS c2;
+(1 row)
+
-- clean up all the random objects we made above
set client_min_messages = warning;
DROP SCHEMA temp_view_test CASCADE;
diff --git a/src/test/regress/sql/create_view.sql b/src/test/regress/sql/create_view.sql
index 6ea6b98708a..ee5feb434c3 100644
--- a/src/test/regress/sql/create_view.sql
+++ b/src/test/regress/sql/create_view.sql
@@ -204,6 +204,17 @@ select * from tt17v;
select pg_get_viewdef('tt17v', true);
select * from int8_tbl i where i.* in (values(i.*::int8_tbl));
+-- check display of ScalarArrayOp with a sub-select
+
+select 'foo'::text = any(array['abc','def','foo']::text[]);
+select 'foo'::text = any((select array['abc','def','foo']::text[])); -- fail
+select 'foo'::text = any((select array['abc','def','foo']::text[])::text[]);
+
+create view tt19v as
+select 'foo'::text = any(array['abc','def','foo']::text[]) c1,
+ 'foo'::text = any((select array['abc','def','foo']::text[])::text[]) c2;
+select pg_get_viewdef('tt19v', true);
+
-- clean up all the random objects we made above
set client_min_messages = warning;
DROP SCHEMA temp_view_test CASCADE;