Re: UNION ALL on partitioned tables won't use indices.

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

In response to

Responses

Browse pgsql-hackers by date

  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