From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | tgl(at)sss(dot)pgh(dot)pa(dot)us |
Cc: | noah(at)leadboat(dot)com, peter_e(at)gmx(dot)net, robertmhaas(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: UNION ALL on partitioned tables won't use indices. |
Date: | 2014-03-04 09:57:56 |
Message-ID: | 20140304.185756.14248729.horiguchi.kyotaro@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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 <noah(at)leadboat(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Yuri Levinsky | 2014-03-04 09:59:02 | requested shared memory size overflows size_t |
Previous Message | Atri Sharma | 2014-03-04 09:51:40 | Re: ALTER TABLE lock strength reduction patch is unsafe |