Re: OUTER JOIN performance regression remains in 8.3beta4

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: OUTER JOIN performance regression remains in 8.3beta4
Date: 2008-01-05 23:38:38
Message-ID: 5139.1199576318@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

I wrote:
> It's possible that we could teach 8.3 to propagate the constant and keep
> the join condition in cases like this;

I think we actually can do this without too big a change. The main
problem is that initsplan.c doesn't put the upper outer join's clause
into the list of mergejoinable outer-join clauses, because it's afraid
that propagating a constant through such a clause might generate a wrong
answer. The case that we're worried about involves propagating an
equal-to-a-constant constraint into the inner (nullable) side of a lower
outer join; this might result in the lower join generating null-extended
rows that should not have appeared in its result (because the rows on
its outer side actually did have matches in the inner side, but those
matches were suppressed by removal of rows not matching the constant).
However, I think this is all right as long as (1) the upper join's clause
is strict, and (2) we still apply the upper join's clause as such,
rather than discarding it. The upper clause will reject the
null-extended rows so it doesn't matter that they really shouldn't have
looked the way they did.

Anyone see any flaws in that reasoning?

To implement this, we should allow distribute_qual_to_rels to put
mergejoinable outer-join clauses into the left/right_join_clauses lists,
even if check_outerjoin_delay returned TRUE for them. However the
result of check_outerjoin_delay has to be made available to
reconsider_outer_join_clauses, so that it will know whether it can
discard clauses or not. The easiest way to do that is to pass it as the
RestrictInfo's outerjoin_delayed flag. This is effectively a small
change in the meaning of outerjoin_delayed: it now is true if the clause
is affected by any *lower* outer join, and so it isn't automatically set
true for an outer-join clause. The old meaning can now be computed as
"outerjoin_delayed || !is_pushed_down", since these days is_pushed_down
is false for exactly those clauses that are non-degenerated outer join
clauses. (Maybe we should rename that flag ... but not right now.)
There is actually only one place in the system that is testing
outerjoin_delayed, so this is an easy change.

Then in reconsider_outer_join_clauses, we can propagate constants
through outer-join clauses if the clause is either strict or not
outerjoin_delayed. However, after a successful propagation, we can drop
the original clause only if it isn't outerjoin_delayed (otherwise we
must keep it to suppress nulls).

Also, reconsider_outer_join_clauses has to be fixed so that if it
successfully deduces any constant constraints, it makes another pass
over the list of outer-join clauses, stopping only when a pass makes no
progress. This is because when we deduce INNERVAR = CONSTANT, we might
now have another clause where INNERVAR is the outer side, and we could
push the constant across to the lower join's inner side. In 8.2 we
accomplished that by having sub_generate_join_implications recurse after
making a deduction, but at the moment 8.3 is failing to do any such
thing. (Hmm, maybe a cheaper fix is to order the outer-join clauses
highest-to-lowest in the list to start with? This would mean merging
the three lists into one, though, which seems to require adding a field
to RestrictInfo so that we can tell what's what.)

Comments?

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message tomas 2008-01-06 05:57:36 Re: Dynamic Partitioning using Segment Visibility Maps
Previous Message Robert Treat 2008-01-05 21:30:04 Re: Dynamic Partitioning using Segment Visibility Maps

Browse pgsql-patches by date

  From Date Subject
Next Message Clodoaldo 2008-01-06 11:26:07 8.3-beta4, analyze and db owner
Previous Message Peter Eisentraut 2008-01-05 21:05:20 Re: SSL over Unix-domain sockets