Skip to content

Commit e5882f2

Browse files
tglsfdctglsfdc
committed
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>
1 parent 691073b commit e5882f2

File tree

3 files changed

+57
-0
lines changed

3 files changed

+57
-0
lines changed

‎src/backend/utils/adt/ruleutils.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6721,6 +6721,24 @@ get_rule_expr(Node *node, deparse_context *context,
67216721
get_base_element_type(exprType(arg2))),
67226722
expr->useOr ? "ANY" : "ALL");
67236723
get_rule_expr_paren(arg2, context, true, node);
6724+
6725+
/*
6726+
* There's inherent ambiguity in "x op ANY/ALL (y)" when y is
6727+
* a bare sub-SELECT. Since we're here, the sub-SELECT must
6728+
* be meant as a scalar sub-SELECT yielding an array value to
6729+
* be used in ScalarArrayOpExpr; but the grammar will
6730+
* preferentially interpret such a construct as an ANY/ALL
6731+
* SubLink. To prevent misparsing the output that way, insert
6732+
* a dummy coercion (which will be stripped by parse analysis,
6733+
* so no inefficiency is added in dump and reload). This is
6734+
* indeed most likely what the user wrote to get the construct
6735+
* accepted in the first place.
6736+
*/
6737+
if (IsA(arg2, SubLink) &&
6738+
((SubLink *) arg2)->subLinkType == EXPR_SUBLINK)
6739+
appendStringInfo(buf, "::%s",
6740+
format_type_with_typemod(exprType(arg2),
6741+
exprTypmod(arg2)));
67246742
appendStringInfoChar(buf, ')');
67256743
if (!PRETTY_PAREN(context))
67266744
appendStringInfoChar(buf, ')');

‎src/test/regress/expected/create_view.out

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1502,6 +1502,34 @@ explain (costs off) select * from tt18v;
15021502
-> Seq Scan on int8_tbl xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx_1
15031503
(3 rows)
15041504

1505+
-- check display of ScalarArrayOp with a sub-select
1506+
select 'foo'::text = any(array['abc','def','foo']::text[]);
1507+
?column?
1508+
----------
1509+
t
1510+
(1 row)
1511+
1512+
select 'foo'::text = any((select array['abc','def','foo']::text[])); -- fail
1513+
ERROR: operator does not exist: text = text[]
1514+
LINE 1: select 'foo'::text = any((select array['abc','def','foo']::t...
1515+
^
1516+
HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
1517+
select 'foo'::text = any((select array['abc','def','foo']::text[])::text[]);
1518+
?column?
1519+
----------
1520+
t
1521+
(1 row)
1522+
1523+
create view tt19v as
1524+
select 'foo'::text = any(array['abc','def','foo']::text[]) c1,
1525+
'foo'::text = any((select array['abc','def','foo']::text[])::text[]) c2;
1526+
select pg_get_viewdef('tt19v', true);
1527+
pg_get_viewdef
1528+
------------------------------------------------------------------------------------------------------------
1529+
SELECT 'foo'::text = ANY (ARRAY['abc'::text, 'def'::text, 'foo'::text]) AS c1, +
1530+
'foo'::text = ANY ((( SELECT ARRAY['abc'::text, 'def'::text, 'foo'::text] AS "array"))::text[]) AS c2;
1531+
(1 row)
1532+
15051533
-- clean up all the random objects we made above
15061534
set client_min_messages = warning;
15071535
DROP SCHEMA temp_view_test CASCADE;

‎src/test/regress/sql/create_view.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,17 @@ create view tt18v as
496496
select pg_get_viewdef('tt18v', true);
497497
explain (costs off) select * from tt18v;
498498

499+
-- check display of ScalarArrayOp with a sub-select
500+
501+
select 'foo'::text = any(array['abc','def','foo']::text[]);
502+
select 'foo'::text = any((select array['abc','def','foo']::text[])); -- fail
503+
select 'foo'::text = any((select array['abc','def','foo']::text[])::text[]);
504+
505+
create view tt19v as
506+
select 'foo'::text = any(array['abc','def','foo']::text[]) c1,
507+
'foo'::text = any((select array['abc','def','foo']::text[])::text[]) c2;
508+
select pg_get_viewdef('tt19v', true);
509+
499510
-- clean up all the random objects we made above
500511
set client_min_messages = warning;
501512
DROP SCHEMA temp_view_test CASCADE;

0 commit comments

Comments
 (0)