database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-06 07:49:08 +03:00
Code Activity

Revisit handling of UNION ALL subqueries with non-Var output columns.

Browse Source 
In commit 57664ed25e 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 b28ffd0fcc 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.
This commit is contained in:
Tom Lane
2012-03-16 13:11:12 -04:00
parent aef5fe7efe
commit dd4134ea56
16 changed files with 412 additions and 259 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
src/backend/optimizer/prep/prepjointree.c
View File

@@ -866,15 +866,22 @@ 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, too.
* We do it this way because we must preserve the AppendRelInfo structs.
* 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.)
*/
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;
}
/*
@@ -1482,31 +1489,14 @@ pullup_replace_vars_callback(Var *var,
if (newnode && IsA(newnode, Var) &&
((Var *) newnode)->varlevelsup == 0)
{
/*
* 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);
/* Simple Vars always escape being wrapped */
wrap = false;
}
else if (newnode && IsA(newnode, PlaceHolderVar) &&
((PlaceHolderVar *) newnode)->phlevelsup == 0)
{
/*
* 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);
/* No need to wrap a PlaceHolderVar with another one, either */
wrap = false;
}
else if (rcon->wrap_non_vars)
{