diff options
| author | Tom Lane | 2016-09-27 22:43:36 +0000 |
|---|---|---|
| committer | Tom Lane | 2016-09-27 22:43:36 +0000 |
| commit | 72daabc7a3e75788df862104b8f723513c2471ae (patch) | |
| tree | 2643e02e8f94bf48f74921c23b3ab87412421752 /src/test | |
| parent | 0109ab27609c0d58c1eddc6b799077d0968083de (diff) | |
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 a4c35ea1c, it would
be much more expensive (or else messier) in older branches.
Per report from Tom van Tilburg.
Discussion: <CAP3PPDiucxYCNev52=YPVkrQAPVF1C5PFWnrQPT7iMzO1fiKFQ@mail.gmail.com>
Diffstat (limited to 'src/test')
| -rw-r--r-- | src/test/regress/expected/subselect.out | 100 | ||||
| -rw-r--r-- | src/test/regress/sql/subselect.sql | 44 |
2 files changed, 144 insertions, 0 deletions
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index 0fc93d9d726..eda319d24b5 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -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); diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index 29912230891..08eb825c542 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -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); |
