diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2012-03-16 13:11:12 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2012-03-16 13:11:55 -0400 |
commit | dd4134ea56cb8855aad3988febc45eca28851cd8 (patch) | |
tree | 4548567dc38ede74d56247a78284a426fc0e8c6f /src/backend/optimizer/util | |
parent | aef5fe7efee5bde4abd618adbaf4c13f44ee59ab (diff) | |
download | postgresql-dd4134ea56cb8855aad3988febc45eca28851cd8.tar.gz postgresql-dd4134ea56cb8855aad3988febc45eca28851cd8.zip |
Revisit handling of UNION ALL subqueries with non-Var output columns.
In commit 57664ed25e5dea117158a2e663c29e60b3546e1c I tried to fix a bug
reported by Teodor Sigaev by making non-simple-Var output columns distinct
(by wrapping their expressions with dummy PlaceHolderVar nodes). This did
not work too well. Commit b28ffd0fcc583c1811e5295279e7d4366c3cae6c fixed
some ensuing problems with matching to child indexes, but per a recent
report from Claus Stadler, constraint exclusion of UNION ALL subqueries was
still broken, because constant-simplification didn't handle the injected
PlaceHolderVars well either. On reflection, the original patch was quite
misguided: there is no reason to expect that EquivalenceClass child members
will be distinct. So instead of trying to make them so, we should ensure
that we can cope with the situation when they're not.
Accordingly, this patch reverts the code changes in the above-mentioned
commits (though the regression test cases they added stay). Instead, I've
added assorted defenses to make sure that duplicate EC child members don't
cause any problems. Teodor's original problem ("MergeAppend child's
targetlist doesn't match MergeAppend") is addressed more directly by
revising prepare_sort_from_pathkeys to let the parent MergeAppend's sort
list guide creation of each child's sort list.
In passing, get rid of add_sort_column; as far as I can tell, testing for
duplicate sort keys at this stage is dead code. Certainly it doesn't
trigger often enough to be worth expending cycles on in ordinary queries.
And keeping the test would've greatly complicated the new logic in
prepare_sort_from_pathkeys, because comparing pathkey list entries against
a previous output array requires that we not skip any entries in the list.
Back-patch to 9.1, like the previous patches. The only known issue in
this area that wasn't caused by the ill-advised previous patches was the
MergeAppend planning failure, which of course is not relevant before 9.1.
It's possible that we need some of the new defenses against duplicate child
EC entries in older branches, but until there's some clear evidence of that
I'm going to refrain from back-patching further.
Diffstat (limited to 'src/backend/optimizer/util')
-rw-r--r-- | src/backend/optimizer/util/placeholder.c | 27 |
1 files changed, 7 insertions, 20 deletions
diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c index 44a41e3e9b8..93f1c2cdfa4 100644 --- a/src/backend/optimizer/util/placeholder.c +++ b/src/backend/optimizer/util/placeholder.c @@ -104,41 +104,28 @@ find_placeholder_info(PlannerInfo *root, PlaceHolderVar *phv, } /* - * find_placeholders_in_query - * Search the query for PlaceHolderVars, and build PlaceHolderInfos + * find_placeholders_in_jointree + * Search the jointree for PlaceHolderVars, and build PlaceHolderInfos * - * 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. + * 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. */ void -find_placeholders_in_query(PlannerInfo *root) +find_placeholders_in_jointree(PlannerInfo *root) { /* We need do nothing if the query contains no PlaceHolderVars */ if (root->glob->lastPHId != 0) { - /* Recursively search the jointree */ + /* Start recursion at top of 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 jointree search for find_placeholders_in_query. + * One recursion level of find_placeholders_in_jointree. * * jtnode is the current jointree node to examine. * |