Hello, I examined the your patch and it seemed reasonable, but I have one question about this patch.
You made ec_relids differ to the union of all ec members' em_relids. Is it right? ==== At Mon, 03 Mar 2014 14:05:10 -0500, Tom Lane wrote > Noah Misch <n...@leadboat.com> writes: > > If you are convinced that a separate flattening pass is best, that suffices > > for me at this stage. Please submit the patch you have in mind, > > incorporating > > any improvements from the v7 patch that are relevant to both approaches. > > I went back and re-read the original message, and this time it struck me > that really the issue here is that add_child_rel_equivalences() doesn't > think it has to deal with the case of a "parent" rel being itself a child. Yes, that is what I tried to solve in one of the first patch but the brute way led to failure:( > That's not inherent though; it's just that it didn't occur to me at the > time that such a situation could arise. It takes only very small changes > to allow that to happen. I saw what you did. I marked not-full-fledged eq members as 'not child'(its real meaning there was full-fledged in contradiction) to reach the child rels. In contrast, you loosened the ec_relids shortcut filter and it seems to work but.. Is it right to make ec_relids different to union of all members' em_relids? This seems to affect joins afterwards. > If you do that, as in the attached changes to equivclass.c, you get > "could not find pathkey item to sort" errors from createplan.c; but > that's just because create_merge_append_plan() is likewise not expecting > the mergeappend's parent rel to be itself a child. Fix for that is > a one-liner, ie, pass down relids. This seems to just correcting the over simplification by the assumption that the best_path there cannot have a parent. > That gets you to a point where the code generates a valid plan, but it's > got nested MergeAppends, which are unnecessary expense. Kyotaro-san's > original fix for that was overcomplicated. It's sufficient to teach > accumulate_append_subpath() to flatten nested MergeAppendPaths. Ah, it was in typ1-collapse patch. As you might noticed seeing there, I couldn't put the assumption that directly nested MergeAppendPaths share the same pathkeys (really?). It's no problem if the assumption goes on. > In short, the attached patch seems to fix it, for a lot less added > complication than anything else that's been discussed on this thread. > I've only lightly tested it (it could use a new regression test case), > but unless someone can find a flaw I think we should use this approach. Mmm. That's motifying but you seems to be right :) Equipping this with some regression tests become my work from now. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers