diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 173f0d21fe..f982aeb049 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -1323,6 +1323,59 @@ inheritance_planner(PlannerInfo *root) child_rte->securityQuals = parent_rte->securityQuals; parent_rte->securityQuals = NIL; + /* + * Mark whether we're planning a query to a partitioned table or an + * inheritance parent. + */ + subroot->inhTargetKind = + partitioned_relids ? INHKIND_PARTITIONED : INHKIND_INHERITED; + + /* + * If this child is further partitioned, remember it as a parent. + * Since a partitioned table does not have any data, we don't need to + * create a plan for it, and we can stop processing it here. We do, + * however, need to remember its modified PlannerInfo for use when + * processing its children, since we'll update their varnos based on + * the delta from immediate parent to child, not from top to child. + * + * Note: a very non-obvious point is that we have not yet added + * duplicate subquery RTEs to the subroot's rtable. We mustn't, + * because then its children would have two sets of duplicates, + * confusing matters. + */ + if (child_rte->inh) + { + Assert(child_rte->relkind == RELKIND_PARTITIONED_TABLE); + parent_relids = bms_add_member(parent_relids, appinfo->child_relid); + parent_roots[appinfo->child_relid] = subroot; + + continue; + } + + /* + * Set the nominal target relation of the ModifyTable node if not + * already done. We use the inheritance parent RTE as the nominal + * target relation if it's a partitioned table (see just above this + * loop). In the non-partitioned parent case, we'll use the first + * child relation (even if it's excluded) as the nominal target + * relation. Because of the way expand_inherited_rtentry works, the + * latter should be the RTE representing the parent table in its role + * as a simple member of the inheritance set. + * + * It would be logically cleaner to *always* use the inheritance + * parent RTE as the nominal relation; but that RTE is not otherwise + * referenced in the plan in the non-partitioned inheritance case. + * Instead the duplicate child RTE created by expand_inherited_rtentry + * is used elsewhere in the plan, so using the original parent RTE + * would give rise to confusing use of multiple aliases in EXPLAIN + * output for what the user will think is the "same" table. OTOH, + * it's not a problem in the partitioned inheritance case, because the + * duplicate child RTE added for the parent does not appear anywhere + * else in the plan tree. + */ + if (nominalRelation < 0) + nominalRelation = appinfo->child_relid; + /* * The rowMarks list might contain references to subquery RTEs, so * make a copy that we can apply ChangeVarNodes to. (Fortunately, the @@ -1426,56 +1479,9 @@ inheritance_planner(PlannerInfo *root) /* and we haven't created PlaceHolderInfos, either */ Assert(subroot->placeholder_list == NIL); - /* - * Mark if we're planning a query to a partitioned table or an - * inheritance parent. - */ - subroot->inhTargetKind = - partitioned_relids ? INHKIND_PARTITIONED : INHKIND_INHERITED; - - /* - * If the child is further partitioned, remember it as a parent. Since - * a partitioned table does not have any data, we don't need to create - * a plan for it. We do, however, need to remember the PlannerInfo for - * use when processing its children. - */ - if (child_rte->inh) - { - Assert(child_rte->relkind == RELKIND_PARTITIONED_TABLE); - parent_relids = - bms_add_member(parent_relids, appinfo->child_relid); - parent_roots[appinfo->child_relid] = subroot; - - continue; - } - /* Generate Path(s) for accessing this result relation */ grouping_planner(subroot, true, 0.0 /* retrieve all tuples */ ); - /* - * Set the nomimal target relation of the ModifyTable node if not - * already done. We use the inheritance parent RTE as the nominal - * target relation if it's a partitioned table (see just above this - * loop). In the non-partitioned parent case, we'll use the first - * child relation (even if it's excluded) as the nominal target - * relation. Because of the way expand_inherited_rtentry works, the - * latter should be the RTE representing the parent table in its role - * as a simple member of the inheritance set. - * - * It would be logically cleaner to *always* use the inheritance - * parent RTE as the nominal relation; but that RTE is not otherwise - * referenced in the plan in the non-partitioned inheritance case. - * Instead the duplicate child RTE created by expand_inherited_rtentry - * is used elsewhere in the plan, so using the original parent RTE - * would give rise to confusing use of multiple aliases in EXPLAIN - * output for what the user will think is the "same" table. OTOH, - * it's not a problem in the partitioned inheritance case, because the - * duplicate child RTE added for the parent does not appear anywhere - * else in the plan tree. - */ - if (nominalRelation < 0) - nominalRelation = appinfo->child_relid; - /* * Select cheapest path in case there's more than one. We always run * modification queries to conclusion, so we care only for the @@ -1493,9 +1499,9 @@ inheritance_planner(PlannerInfo *root) continue; /* - * Add the current parent's RT index to the partitione_rels set if - * we're going to create the ModifyTable path for a partitioned root - * table. + * Add the current parent's RT index to the partitioned_relids set if + * we're creating the ModifyTable path for a partitioned root table. + * (We only care about parents of non-excluded children.) */ if (partitioned_relids) partitioned_relids = bms_add_member(partitioned_relids, diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out index b983f9c506..ae552eb362 100644 --- a/src/test/regress/expected/partition_join.out +++ b/src/test/regress/expected/partition_join.out @@ -1652,6 +1652,49 @@ SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1_l WHERE a = 1 AND a = 2) One-Time Filter: false (11 rows) +-- Test case to verify proper handling of subqueries in a partitioned delete. +-- The weird-looking lateral join is just there to force creation of a +-- nestloop parameter within the subquery, which exposes the problem if the +-- planner fails to make multiple copies of the subquery as appropriate. +EXPLAIN (COSTS OFF) +DELETE FROM prt1_l +WHERE EXISTS ( + SELECT 1 + FROM int4_tbl, + LATERAL (SELECT int4_tbl.f1 FROM int8_tbl LIMIT 2) ss + WHERE prt1_l.c IS NULL); + QUERY PLAN +--------------------------------------------------------------- + Delete on prt1_l + Delete on prt1_l_p1 + Delete on prt1_l_p3_p1 + Delete on prt1_l_p3_p2 + -> Nested Loop Semi Join + -> Seq Scan on prt1_l_p1 + Filter: (c IS NULL) + -> Nested Loop + -> Seq Scan on int4_tbl + -> Subquery Scan on ss + -> Limit + -> Seq Scan on int8_tbl + -> Nested Loop Semi Join + -> Seq Scan on prt1_l_p3_p1 + Filter: (c IS NULL) + -> Nested Loop + -> Seq Scan on int4_tbl + -> Subquery Scan on ss_1 + -> Limit + -> Seq Scan on int8_tbl int8_tbl_1 + -> Nested Loop Semi Join + -> Seq Scan on prt1_l_p3_p2 + Filter: (c IS NULL) + -> Nested Loop + -> Seq Scan on int4_tbl + -> Subquery Scan on ss_2 + -> Limit + -> Seq Scan on int8_tbl int8_tbl_2 +(28 rows) + -- -- negative testcases -- diff --git a/src/test/regress/sql/partition_join.sql b/src/test/regress/sql/partition_join.sql index a2d8b1be55..32d4927409 100644 --- a/src/test/regress/sql/partition_join.sql +++ b/src/test/regress/sql/partition_join.sql @@ -319,6 +319,18 @@ SELECT * FROM prt1_l t1 LEFT JOIN LATERAL EXPLAIN (COSTS OFF) SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1_l WHERE a = 1 AND a = 2) t1 RIGHT JOIN prt2_l t2 ON t1.a = t2.b AND t1.b = t2.a AND t1.c = t2.c; +-- Test case to verify proper handling of subqueries in a partitioned delete. +-- The weird-looking lateral join is just there to force creation of a +-- nestloop parameter within the subquery, which exposes the problem if the +-- planner fails to make multiple copies of the subquery as appropriate. +EXPLAIN (COSTS OFF) +DELETE FROM prt1_l +WHERE EXISTS ( + SELECT 1 + FROM int4_tbl, + LATERAL (SELECT int4_tbl.f1 FROM int8_tbl LIMIT 2) ss + WHERE prt1_l.c IS NULL); + -- -- negative testcases --