Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 1f7c85b

Browse files
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 c4a586c commit 1f7c85b

File tree

3 files changed

+57
-0
lines changed

3 files changed

+57
-0
lines changed

src/backend/utils/adt/ruleutils.c

+18
Original file line numberDiff line numberDiff line change
@@ -7209,6 +7209,24 @@ get_rule_expr(Node *node, deparse_context *context,
72097209
get_base_element_type(exprType(arg2))),
72107210
expr->useOr ? "ANY" : "ALL");
72117211
get_rule_expr_paren(arg2, context, true, node);
7212+
7213+
/*
7214+
* There's inherent ambiguity in "x op ANY/ALL (y)" when y is
7215+
* a bare sub-SELECT. Since we're here, the sub-SELECT must
7216+
* be meant as a scalar sub-SELECT yielding an array value to
7217+
* be used in ScalarArrayOpExpr; but the grammar will
7218+
* preferentially interpret such a construct as an ANY/ALL
7219+
* SubLink. To prevent misparsing the output that way, insert
7220+
* a dummy coercion (which will be stripped by parse analysis,
7221+
* so no inefficiency is added in dump and reload). This is
7222+
* indeed most likely what the user wrote to get the construct
7223+
* accepted in the first place.
7224+
*/
7225+
if (IsA(arg2, SubLink) &&
7226+
((SubLink *) arg2)->subLinkType == EXPR_SUBLINK)
7227+
appendStringInfo(buf, "::%s",
7228+
format_type_with_typemod(exprType(arg2),
7229+
exprTypmod(arg2)));
72127230
appendStringInfoChar(buf, ')');
72137231
if (!PRETTY_PAREN(context))
72147232
appendStringInfoChar(buf, ')');

src/test/regress/expected/create_view.out

+28
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

+11
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)