Re: BUG #11457: The below query crashes 9.3.5, but not 9.3.4

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Nelson Page <npage(at)dynamicsignal(dot)com>, "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #11457: The below query crashes 9.3.5, but not 9.3.4
Date: 2014-10-01 05:43:28
Message-ID: 966.1412142208@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> Attached is an improved version of this patch with better names for
> the new functions and variables. I added more comments at the same
> time.

After studying this a bit more, I've gotten disillusioned with the
idea of precalculating the top_parent_relid for an AppendRelInfo.

1. It's too complicated to maintain. Your patch misses out doing
something when expand_inherited_rtentry() creates a child rel of
a rel that's already somebody else's child, and much harder to fix,
it misses out doing something when pull_up_simple_subquery() merges
a child query's append_rel_list with the parent query's. That
action could well result in some of the pulled-up child AppendRelInfos
now being children of pre-existing parent AppendRelInfos.
These things could probably be fixed, but:

2. There's no evidence that we actually have a performance problem we
need to fix in this area. Multilevel parentage is pretty rare, else
we'd have noticed problems here before. By the time we fix #1, we could
easily waste more cycles maintaining the data structure than we save.

3. Keeping the topmost parent relid only helps in some places anyhow.
In particular, in generate_join_implied_equalities_broken, we really
have to apply all the translation steps down from the top rel. (It
took me several hours of fooling around to generate a test case for
this ... but the attached patch includes a new regression test file
that exercises that code, and it shows a failure both with HEAD and
with your patch.)

4. Also, in generate_implied_equalities_for_column and
check_partial_indexes, it seems prudent to me to exclude all appendrel
parents of the child we're considering, not only the topmost. This is
probably moot at the moment but it might not be so forever, in particular
if we ever get around to fixing the problem that sub-SELECTs containing
WHERE clauses can't be pulled up as appendrels. (That'd result in WHERE
clauses associated with intermediate-level appendrels, and I think that
might lead us to generate bogus join paths in the same way as the current
problem does.)

Accordingly, I propose a patch more like the attached. This doesn't
try to change the data structures, but it does take the viewpoint that
all current callers of find_childrel_appendrelinfo() need to be fixed
to explicitly consider multiple levels of parent appendrels.

regards, tom lane

Attachment Content-Type Size
nested-appendrels-fixes-1.patch text/x-diff 31.3 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Michael Paquier 2014-10-01 07:52:43 Re: BUG #11457: The below query crashes 9.3.5, but not 9.3.4
Previous Message Michael Paquier 2014-10-01 04:11:55 Re: BUG #11457: The below query crashes 9.3.5, but not 9.3.4