From 555494d1bc119173bbf712940da823303644d4de Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 2 Feb 2017 19:11:27 -0500 Subject: [PATCH] Fix placement of initPlans when forcibly materializing a subplan. If we forcibly place a Material node atop a finished subplan, we need to move any initPlans attached to the subplan up to the Material node, in order to keep SS_finalize_plan() happy. I'd figured this out in commit 7b67a0a49 for the case of materializing a cursor plan, but out of an abundance of caution, I put the initPlan movement hack at the call site for that case, rather than inside materialize_finished_plan(). That was the wrong thing, because it turns out to also be necessary for the only other caller of materialize_finished_plan(), ie subselect.c. We lacked any test cases that exposed the mistake, but bug#14524 from Wei Congrui shows that it's possible to get an initPlan reference into the top tlist in that case too, and then SS_finalize_plan() complains. Hence, move the hack into materialize_finished_plan(). In HEAD, also relocate some recently-added tests in subselect.sql, which I'd unthinkingly dropped into the middle of a sequence of related tests. Report: https://postgr.es/m/20170202060020.1400.89021@wrigleys.postgresql.org --- src/backend/optimizer/plan/createplan.c | 10 ++++ src/backend/optimizer/plan/planner.c | 16 +----- src/test/regress/expected/subselect.out | 69 ++++++++++++++++--------- src/test/regress/sql/subselect.sql | 25 +++++---- 4 files changed, 72 insertions(+), 48 deletions(-) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index fae1f67b9c..997bdcff2e 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -5704,6 +5704,16 @@ materialize_finished_plan(Plan *subplan) matplan = (Plan *) make_material(subplan); + /* + * XXX horrid kluge: if there are any initPlans attached to the subplan, + * move them up to the Material node, which is now effectively the top + * plan node in its query level. This prevents failure in + * SS_finalize_plan(), which see for comments. We don't bother adjusting + * the subplan's cost estimate for this. + */ + matplan->initPlan = subplan->initPlan; + subplan->initPlan = NIL; + /* Set cost data */ cost_material(&matpath, subplan->startup_cost, diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 4b5902fc3e..881742f46b 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -316,21 +316,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) if (cursorOptions & CURSOR_OPT_SCROLL) { if (!ExecSupportsBackwardScan(top_plan)) - { - Plan *sub_plan = top_plan; - - top_plan = materialize_finished_plan(sub_plan); - - /* - * XXX horrid kluge: if there are any initPlans attached to the - * formerly-top plan node, move them up to the Material node. This - * prevents failure in SS_finalize_plan, which see for comments. - * We don't bother adjusting the sub_plan's cost estimate for - * this. - */ - top_plan->initPlan = sub_plan->initPlan; - sub_plan->initPlan = NIL; - } + top_plan = materialize_finished_plan(top_plan); } /* diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index 1ad4ad47b2..ed7d6d8034 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -196,6 +196,31 @@ SELECT '' AS five, f1 AS "Correlated Field" | 3 (5 rows) +-- +-- Use some existing tables in the regression test +-- +SELECT '' AS eight, ss.f1 AS "Correlated Field", ss.f3 AS "Second Field" + FROM SUBSELECT_TBL ss + WHERE f1 NOT IN (SELECT f1+1 FROM INT4_TBL + WHERE f1 != ss.f1 AND f1 < 2147483647); + eight | Correlated Field | Second Field +-------+------------------+-------------- + | 2 | 4 + | 3 | 5 + | 2 | 2 + | 3 | 3 + | 6 | 8 + | 8 | +(6 rows) + +select q1, float8(count(*)) / (select count(*) from int8_tbl) +from int8_tbl group by q1 order by q1; + q1 | ?column? +------------------+---------- + 123 | 0.4 + 4567890123456789 | 0.6 +(2 rows) + -- Unspecified-type literals in output columns should resolve as text SELECT *, pg_typeof(f1) FROM (SELECT 'foo' AS f1 FROM generate_series(1,3)) ss ORDER BY 1; @@ -227,30 +252,28 @@ explain verbose select '42' union all select 43; Output: 43 (5 rows) --- --- Use some existing tables in the regression test --- -SELECT '' AS eight, ss.f1 AS "Correlated Field", ss.f3 AS "Second Field" - FROM SUBSELECT_TBL ss - WHERE f1 NOT IN (SELECT f1+1 FROM INT4_TBL - WHERE f1 != ss.f1 AND f1 < 2147483647); - eight | Correlated Field | Second Field --------+------------------+-------------- - | 2 | 4 - | 3 | 5 - | 2 | 2 - | 3 | 3 - | 6 | 8 - | 8 | -(6 rows) +-- check materialization of an initplan reference (bug #14524) +explain (verbose, costs off) +select 1 = all (select (select 1)); + QUERY PLAN +----------------------------------- + Result + Output: (SubPlan 2) + SubPlan 2 + -> Materialize + Output: ($0) + InitPlan 1 (returns $0) + -> Result + Output: 1 + -> Result + Output: $0 +(10 rows) -select q1, float8(count(*)) / (select count(*) from int8_tbl) -from int8_tbl group by q1 order by q1; - q1 | ?column? -------------------+---------- - 123 | 0.4 - 4567890123456789 | 0.6 -(2 rows) +select 1 = all (select (select 1)); + ?column? +---------- + t +(1 row) -- -- Check EXISTS simplification with LIMIT diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index 9c2a73d4d7..2fc0e26ca0 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -80,16 +80,6 @@ SELECT '' AS five, f1 AS "Correlated Field" WHERE (f1, f2) IN (SELECT f2, CAST(f3 AS int4) FROM SUBSELECT_TBL WHERE f3 IS NOT NULL); --- Unspecified-type literals in output columns should resolve as text - -SELECT *, pg_typeof(f1) FROM - (SELECT 'foo' AS f1 FROM generate_series(1,3)) ss ORDER BY 1; - --- ... unless there's context to suggest differently - -explain verbose select '42' union all select '43'; -explain verbose select '42' union all select 43; - -- -- Use some existing tables in the regression test -- @@ -102,6 +92,21 @@ SELECT '' AS eight, ss.f1 AS "Correlated Field", ss.f3 AS "Second Field" select q1, float8(count(*)) / (select count(*) from int8_tbl) from int8_tbl group by q1 order by q1; +-- Unspecified-type literals in output columns should resolve as text + +SELECT *, pg_typeof(f1) FROM + (SELECT 'foo' AS f1 FROM generate_series(1,3)) ss ORDER BY 1; + +-- ... unless there's context to suggest differently + +explain verbose select '42' union all select '43'; +explain verbose select '42' union all select 43; + +-- check materialization of an initplan reference (bug #14524) +explain (verbose, costs off) +select 1 = all (select (select 1)); +select 1 = all (select (select 1)); + -- -- Check EXISTS simplification with LIMIT -- -- 2.40.0