diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c index 55311058e5..4c076722de 100644 --- a/src/backend/optimizer/plan/planmain.c +++ b/src/backend/optimizer/plan/planmain.c @@ -176,7 +176,7 @@ query_planner(PlannerInfo *root, List *tlist, */ build_base_rel_tlists(root, tlist); - find_placeholders_in_jointree(root); + find_placeholders_in_query(root); joinlist = deconstruct_jointree(root); diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index aeaae8c8d8..8bb011b711 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -784,22 +784,15 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, parse->havingQual = pullup_replace_vars(parse->havingQual, &rvcontext); /* - * Replace references in the translated_vars lists of appendrels. When - * pulling up an appendrel member, we do not need PHVs in the list of the - * parent appendrel --- there isn't any outer join between. Elsewhere, use - * PHVs for safety. (This analysis could be made tighter but it seems - * unlikely to be worth much trouble.) + * Replace references in the translated_vars lists of appendrels, too. + * We do it this way because we must preserve the AppendRelInfo structs. */ foreach(lc, root->append_rel_list) { AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc); - bool save_need_phvs = rvcontext.need_phvs; - if (appinfo == containing_appendrel) - rvcontext.need_phvs = false; appinfo->translated_vars = (List *) pullup_replace_vars((Node *) appinfo->translated_vars, &rvcontext); - rvcontext.need_phvs = save_need_phvs; } /* @@ -1407,14 +1400,31 @@ pullup_replace_vars_callback(Var *var, if (newnode && IsA(newnode, Var) && ((Var *) newnode)->varlevelsup == 0) { - /* Simple Vars always escape being wrapped */ - wrap = false; + /* + * Simple Vars normally escape being wrapped. However, in + * wrap_non_vars mode (ie, we are dealing with an appendrel + * member), we must ensure that each tlist entry expands to a + * distinct expression, else we may have problems with + * improperly placing identical entries into different + * EquivalenceClasses. Therefore, we wrap a Var in a + * PlaceHolderVar if it duplicates any earlier entry in the + * tlist (ie, we've got "SELECT x, x, ..."). Since each PHV + * is distinct, this fixes the ambiguity. We can use + * tlist_member to detect whether there's an earlier + * duplicate. + */ + wrap = (rcon->wrap_non_vars && + tlist_member(newnode, rcon->targetlist) != tle); } else if (newnode && IsA(newnode, PlaceHolderVar) && ((PlaceHolderVar *) newnode)->phlevelsup == 0) { - /* No need to wrap a PlaceHolderVar with another one, either */ - wrap = false; + /* + * No need to directly wrap a PlaceHolderVar with another one, + * either, unless we need to prevent duplication. + */ + wrap = (rcon->wrap_non_vars && + tlist_member(newnode, rcon->targetlist) != tle); } else if (rcon->wrap_non_vars) { diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c index 8b65245b51..3ae5154684 100644 --- a/src/backend/optimizer/util/placeholder.c +++ b/src/backend/optimizer/util/placeholder.c @@ -104,28 +104,41 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv, } /* - * find_placeholders_in_jointree - * Search the jointree for PlaceHolderVars, and build PlaceHolderInfos + * find_placeholders_in_query + * Search the query for PlaceHolderVars, and build PlaceHolderInfos * - * We don't need to look at the targetlist because build_base_rel_tlists() - * will already have made entries for any PHVs in the tlist. + * We need to examine the jointree, but not the targetlist, because + * build_base_rel_tlists() will already have made entries for any PHVs + * in the targetlist. + * + * We also need to search for PHVs in AppendRelInfo translated_vars + * lists. In most cases, translated_vars entries aren't directly referenced + * elsewhere, but we need to create PlaceHolderInfo entries for them to + * support set_rel_width() calculations for the appendrel child relations. */ void -find_placeholders_in_jointree(PlannerInfo *root) +find_placeholders_in_query(PlannerInfo *root) { /* We need do nothing if the query contains no PlaceHolderVars */ if (root->glob->lastPHId != 0) { - /* Start recursion at top of jointree */ + /* Recursively search the jointree */ Assert(root->parse->jointree != NULL && IsA(root->parse->jointree, FromExpr)); (void) find_placeholders_recurse(root, (Node *) root->parse->jointree); + + /* + * Also search the append_rel_list for translated vars that are PHVs. + * Barring finding them elsewhere in the query, they do not need any + * ph_may_need bits, only to be present in the PlaceHolderInfo list. + */ + mark_placeholders_in_expr(root, (Node *) root->append_rel_list, NULL); } } /* * find_placeholders_recurse - * One recursion level of find_placeholders_in_jointree. + * One recursion level of jointree search for find_placeholders_in_query. * * jtnode is the current jointree node to examine. * diff --git a/src/include/optimizer/placeholder.h b/src/include/optimizer/placeholder.h index f112419fd5..5fbebfcb0c 100644 --- a/src/include/optimizer/placeholder.h +++ b/src/include/optimizer/placeholder.h @@ -21,7 +21,7 @@ extern PlaceHolderVar *make_placeholder_expr(PlannerInfo *root, Expr *expr, Relids phrels); extern PlaceHolderInfo *find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv, bool create_new_ph); -extern void find_placeholders_in_jointree(PlannerInfo *root); +extern void find_placeholders_in_query(PlannerInfo *root); extern void mark_placeholder_maybe_needed(PlannerInfo *root, PlaceHolderInfo *phinfo, Relids relids); extern void update_placeholder_eval_levels(PlannerInfo *root, diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 0948974d9a..3e2ab9ac5b 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1242,3 +1242,61 @@ NOTICE: drop cascades to 3 other objects DETAIL: drop cascades to table matest1 drop cascades to table matest2 drop cascades to table matest3 +-- +-- Test merge-append for UNION ALL append relations +-- Check handling of duplicated, constant, or volatile targetlist items +-- +set enable_seqscan = off; +set enable_indexscan = on; +set enable_bitmapscan = off; +explain (costs off) +SELECT thousand, tenthous FROM tenk1 +UNION ALL +SELECT thousand, thousand FROM tenk1 +ORDER BY thousand, tenthous; + QUERY PLAN +----------------------------------------------------------------------- + Result + -> Merge Append + Sort Key: public.tenk1.thousand, public.tenk1.tenthous + -> Index Only Scan using tenk1_thous_tenthous on tenk1 + -> Sort + Sort Key: public.tenk1.thousand, public.tenk1.thousand + -> Index Only Scan using tenk1_thous_tenthous on tenk1 +(7 rows) + +explain (costs off) +SELECT thousand, tenthous FROM tenk1 +UNION ALL +SELECT 42, 42 FROM tenk1 +ORDER BY thousand, tenthous; + QUERY PLAN +----------------------------------------------------------------------- + Result + -> Merge Append + Sort Key: public.tenk1.thousand, public.tenk1.tenthous + -> Index Only Scan using tenk1_thous_tenthous on tenk1 + -> Sort + Sort Key: (42), (42) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 +(7 rows) + +explain (costs off) +SELECT thousand, tenthous FROM tenk1 +UNION ALL +SELECT thousand, random()::integer FROM tenk1 +ORDER BY thousand, tenthous; + QUERY PLAN +----------------------------------------------------------------------- + Result + -> Merge Append + Sort Key: public.tenk1.thousand, public.tenk1.tenthous + -> Index Only Scan using tenk1_thous_tenthous on tenk1 + -> Sort + Sort Key: public.tenk1.thousand, ((random())::integer) + -> Index Only Scan using tenk1_thous_tenthous on tenk1 +(7 rows) + +reset enable_seqscan; +reset enable_indexscan; +reset enable_bitmapscan; diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 3087a14b72..6e5a1d1c8e 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -406,3 +406,34 @@ select * from matest0 order by 1-id; reset enable_seqscan; drop table matest0 cascade; + +-- +-- Test merge-append for UNION ALL append relations +-- Check handling of duplicated, constant, or volatile targetlist items +-- + +set enable_seqscan = off; +set enable_indexscan = on; +set enable_bitmapscan = off; + +explain (costs off) +SELECT thousand, tenthous FROM tenk1 +UNION ALL +SELECT thousand, thousand FROM tenk1 +ORDER BY thousand, tenthous; + +explain (costs off) +SELECT thousand, tenthous FROM tenk1 +UNION ALL +SELECT 42, 42 FROM tenk1 +ORDER BY thousand, tenthous; + +explain (costs off) +SELECT thousand, tenthous FROM tenk1 +UNION ALL +SELECT thousand, random()::integer FROM tenk1 +ORDER BY thousand, tenthous; + +reset enable_seqscan; +reset enable_indexscan; +reset enable_bitmapscan;