At Thu, 27 Feb 2014 21:53:52 -0500, Noah Misch wrote
> On Thu, Feb 27, 2014 at 05:33:47PM -0500, Tom Lane wrote:
> > Noah Misch <n...@leadboat.com> writes:
> > > If the attached patch version looks reasonable, I will commit it.
> > 
> > The test case is completely bogus, as the query explained is significantly
> > different from the query executed.  I'm not sure whether you can just
> > remove the extra ORDER BY column without getting machine-dependent
> > results, though.
> Each query tests something slightly different.  The EXPLAIN verifies that we
> can MergeAppend given this query structure, and the plain SELECT verifies that
> any join tree contortions we made to achieve that do not change the answer.

I think Tom said that the second query can yield the same result
even if the 2nd column in orderby is removed so the pertinence of
it seems doubtful. Actually the altered query gave me the same
result on my workset (CentOS6.5/x86-64).

> > More generally, I'm afraid the whole approach is probably wrong, or at
> > least not solving all problems in this area, because of this:
> > 
> > > Incidentally, I tried adding an assertion that append_rel_list does not 
> > > show
> > > one appendrel as a direct child of another.  The following query, 
> > > off-topic
> > > for the patch at hand, triggered that assertion:
> > 
> > That's not "off topic" at all; it shows that there's not been any effort
> > to date to flatten appendrel membership, and therefore this partial
> > implementation is going to miss some cases.  It doesn't help to merge an
> > inheritance-based appendrel into its parent if the query ORDER BY is still
> > a level or two above that due to UNION ALLs.
> Nonetheless, I find it reasonable to accept a patch that makes
> expand_inherited_tables() avoid introducing nested appendrels.  Making
> pull_up_subqueries() do the same can be a separate effort.  There will still
> be a pile of ways to stifle MergeAppend, including everything that makes
> is_simple_union_all() return false.  Having said that, both you and Kyotaro
> sound keen to achieve an appendrel flatness invariant in a single patch.  If
> that's the consensus, I'm fine with bouncing this back so it can happen.

I understood what that query causes and still don't have definite
opinion on which is better between fixing them individually and
in one go. Nontheless, I'd feel more at ease flattening them
altogether regardless of their origin.

> > I wonder whether we should consider adding a pass to flatten any nested
> > appendrels after we're done creating them all.
> We did consider that upthread.  It's a valid option, but I remain more
> inclined to teach pull_up_subqueries() to preserve flatness just like
> expand_inherited_tables() will.

Yes, the old dumped version of typ2 patch did so. It flattened
appendrel tree for the query prpoerly. Let me hear the reson you
prefer to do so.

> > Or alternatively, perhaps
> > rather than changing the representation invariant, we need to take a
> > harder look at why ordering info isn't getting pushed down through
> > appendrels.
> That could facilitate MergeAppend in considerably more places, such as UNION
> ALL containing non-simple branches.  I don't have much of a sense concerning
> what such a patch would entail.


Kyotaro Horiguchi
NTT Open Source Software Center

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to