Skip to content

Commit 2865d59

Browse files
committed
Don't assume a subquery's output is unique if there's a SRF in its tlist.
While the x output of "select x from t group by x" can be presumed unique, this does not hold for "select x, generate_series(1,10) from t group by x", because we may expand the set-returning function after the grouping step. (Perhaps that should be re-thought; but considering all the other oddities involved with SRFs in targetlists, it seems unlikely we'll change it.) Put a check in query_is_distinct_for() so it's not fooled by such cases. Back-patch to all supported branches. David Rowley
1 parent d0091e3 commit 2865d59

File tree

3 files changed

+56
-0
lines changed

3 files changed

+56
-0
lines changed

src/backend/optimizer/util/pathnode.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include "catalog/pg_operator.h"
2020
#include "miscadmin.h"
21+
#include "nodes/nodeFuncs.h"
2122
#include "optimizer/clauses.h"
2223
#include "optimizer/cost.h"
2324
#include "optimizer/pathnode.h"
@@ -1093,6 +1094,17 @@ query_is_distinct_for(Query *query, List *colnos, List *opids)
10931094

10941095
Assert(list_length(colnos) == list_length(opids));
10951096

1097+
/*
1098+
* A set-returning function in the query's targetlist can result in
1099+
* returning duplicate rows, if the SRF is evaluated after the
1100+
* de-duplication step; so we play it safe and say "no" if there are any
1101+
* SRFs. (We could be certain that it's okay if SRFs appear only in the
1102+
* specified columns, since those must be evaluated before de-duplication;
1103+
* but it doesn't presently seem worth the complication to check that.)
1104+
*/
1105+
if (expression_returns_set((Node *) query->targetList))
1106+
return false;
1107+
10961108
/*
10971109
* DISTINCT (including DISTINCT ON) guarantees uniqueness if all the
10981110
* columns in the DISTINCT clause appear in colnos and operator semantics

src/test/regress/expected/subselect.out

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,3 +732,36 @@ select * from int4_tbl where
732732
0
733733
(1 row)
734734

735+
--
736+
-- Check for incorrect optimization when IN subquery contains a SRF
737+
--
738+
set enable_hashjoin to 0;
739+
explain (verbose, costs off)
740+
select * from int4_tbl o where (f1, f1) in
741+
(select f1, generate_series(1,2) / 10 g from int4_tbl i group by f1);
742+
QUERY PLAN
743+
----------------------------------------------------------------
744+
Nested Loop Semi Join
745+
Output: o.f1
746+
Join Filter: (o.f1 = "ANY_subquery".f1)
747+
-> Seq Scan on public.int4_tbl o
748+
Output: o.f1
749+
-> Materialize
750+
Output: "ANY_subquery".f1, "ANY_subquery".g
751+
-> Subquery Scan on "ANY_subquery"
752+
Output: "ANY_subquery".f1, "ANY_subquery".g
753+
Filter: ("ANY_subquery".f1 = "ANY_subquery".g)
754+
-> HashAggregate
755+
Output: i.f1, (generate_series(1, 2) / 10)
756+
-> Seq Scan on public.int4_tbl i
757+
Output: i.f1
758+
(14 rows)
759+
760+
select * from int4_tbl o where (f1, f1) in
761+
(select f1, generate_series(1,2) / 10 g from int4_tbl i group by f1);
762+
f1
763+
----
764+
0
765+
(1 row)
766+
767+
reset enable_hashjoin;

src/test/regress/sql/subselect.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,3 +416,14 @@ select * from int4_tbl where
416416
select * from int4_tbl where
417417
(case when f1 in (select unique1 from tenk1 a) then f1 else null end) in
418418
(select ten from tenk1 b);
419+
420+
--
421+
-- Check for incorrect optimization when IN subquery contains a SRF
422+
--
423+
set enable_hashjoin to 0;
424+
explain (verbose, costs off)
425+
select * from int4_tbl o where (f1, f1) in
426+
(select f1, generate_series(1,2) / 10 g from int4_tbl i group by f1);
427+
select * from int4_tbl o where (f1, f1) in
428+
(select f1, generate_series(1,2) / 10 g from int4_tbl i group by f1);
429+
reset enable_hashjoin;

0 commit comments

Comments
 (0)