• Tom Lane's avatar
    Revisit handling of UNION ALL subqueries with non-Var output columns. · dd4134ea
    Tom Lane authored
    In commit 57664ed2 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 b28ffd0f 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.
    dd4134ea
inherit.out 39 KB