Commit 72daabc7 authored by Tom Lane's avatar Tom Lane

Disallow pushing volatile quals past set-returning functions.

Pushing an upper-level restriction clause into an unflattened
subquery-in-FROM is okay when the subquery contains no SRFs in its
targetlist, or when it does but the SRFs are unreferenced by the clause
*and the clause is not volatile*.  Otherwise, we're changing the number
of times the clause is evaluated, which is bad for volatile quals, and
possibly changing the result, since a volatile qual might succeed for some
SRF output rows and not others despite not referencing any of the changing
columns.  (Indeed, if the clause is something like "random() > 0.5", the
user is probably expecting exactly that behavior.)

We had most of these restrictions down, but not the one about the upper
clause not being volatile.  Fix that, and add a regression test to
illustrate the expected behavior.

Although this is definitely a bug, it doesn't seem like back-patch
material, since possibly some users don't realize that the broken
behavior is broken and are relying on what happens now.  Also, while
the added test is quite cheap in the wake of commit a4c35ea1, it would
be much more expensive (or else messier) in older branches.

Per report from Tom van Tilburg.

Discussion: <CAP3PPDiucxYCNev52=YPVkrQAPVF1C5PFWnrQPT7iMzO1fiKFQ@mail.gmail.com>
parent 0109ab27
......@@ -2254,6 +2254,12 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
* thereby changing the partition contents and thus the window functions'
* results for rows that remain.
*
* 5. If the subquery contains any set-returning functions in its targetlist,
* we cannot push volatile quals into it. That would push them below the SRFs
* and thereby change the number of times they are evaluated. Also, a
* volatile qual could succeed for some SRF output rows and fail for others,
* a behavior that cannot occur if it's evaluated before SRF expansion.
*
* In addition, we make several checks on the subquery's output columns to see
* if it is safe to reference them in pushed-down quals. If output column k
* is found to be unsafe to reference, we set safetyInfo->unsafeColumns[k]
......@@ -2298,8 +2304,10 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
if (subquery->limitOffset != NULL || subquery->limitCount != NULL)
return false;
/* Check points 3 and 4 */
if (subquery->distinctClause || subquery->hasWindowFuncs)
/* Check points 3, 4, and 5 */
if (subquery->distinctClause ||
subquery->hasWindowFuncs ||
subquery->hasTargetSRFs)
safetyInfo->unsafeVolatile = true;
/*
......
......@@ -880,3 +880,103 @@ select nextval('ts1');
11
(1 row)
--
-- Check that volatile quals aren't pushed down past a set-returning function;
-- while a nonvolatile qual can be, if it doesn't reference the SRF.
--
create function tattle(x int, y int) returns bool
volatile language plpgsql as $$
begin
raise notice 'x = %, y = %', x, y;
return x > y;
end$$;
explain (verbose, costs off)
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, 8);
QUERY PLAN
----------------------------------------------------------
Subquery Scan on ss
Output: x, u
Filter: tattle(ss.x, 8)
-> Result
Output: 9, unnest('{1,2,3,11,12,13}'::integer[])
(5 rows)
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, 8);
NOTICE: x = 9, y = 8
NOTICE: x = 9, y = 8
NOTICE: x = 9, y = 8
NOTICE: x = 9, y = 8
NOTICE: x = 9, y = 8
NOTICE: x = 9, y = 8
x | u
---+----
9 | 1
9 | 2
9 | 3
9 | 11
9 | 12
9 | 13
(6 rows)
-- if we pretend it's stable, we get different results:
alter function tattle(x int, y int) stable;
explain (verbose, costs off)
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, 8);
QUERY PLAN
----------------------------------------------------
Result
Output: 9, unnest('{1,2,3,11,12,13}'::integer[])
One-Time Filter: tattle(9, 8)
(3 rows)
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, 8);
NOTICE: x = 9, y = 8
x | u
---+----
9 | 1
9 | 2
9 | 3
9 | 11
9 | 12
9 | 13
(6 rows)
-- although even a stable qual should not be pushed down if it references SRF
explain (verbose, costs off)
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, u);
QUERY PLAN
----------------------------------------------------------
Subquery Scan on ss
Output: x, u
Filter: tattle(ss.x, ss.u)
-> Result
Output: 9, unnest('{1,2,3,11,12,13}'::integer[])
(5 rows)
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, u);
NOTICE: x = 9, y = 1
NOTICE: x = 9, y = 2
NOTICE: x = 9, y = 3
NOTICE: x = 9, y = 11
NOTICE: x = 9, y = 12
NOTICE: x = 9, y = 13
x | u
---+---
9 | 1
9 | 2
9 | 3
(3 rows)
drop function tattle(x int, y int);
......@@ -481,3 +481,47 @@ select * from
order by 1;
select nextval('ts1');
--
-- Check that volatile quals aren't pushed down past a set-returning function;
-- while a nonvolatile qual can be, if it doesn't reference the SRF.
--
create function tattle(x int, y int) returns bool
volatile language plpgsql as $$
begin
raise notice 'x = %, y = %', x, y;
return x > y;
end$$;
explain (verbose, costs off)
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, 8);
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, 8);
-- if we pretend it's stable, we get different results:
alter function tattle(x int, y int) stable;
explain (verbose, costs off)
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, 8);
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, 8);
-- although even a stable qual should not be pushed down if it references SRF
explain (verbose, costs off)
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, u);
select * from
(select 9 as x, unnest(array[1,2,3,11,12,13]) as u) ss
where tattle(x, u);
drop function tattle(x int, y int);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment