From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)postgreSQL(dot)org |
Subject: | Re: Bugs in planner's equivalence-class processing |
Date: | 2012-10-17 23:06:38 |
Message-ID: | 11539.1350515198@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I wrote:
> So this is essentially an oversight in the patch that added tracking of
> nullable_relids. I got confused about the difference between
> outerjoin_delayed (this clause, as a whole, is not outerjoin_delayed
> because its natural semantic level would be at the join anyway) and
> having nonempty nullable_relids, and thought that equivalence-class
> processing would never see such clauses because it doesn't accept
> outerjoin_delayed clauses. So there's no code in equivclass.c to
> compute nullable_relids sets for constructed clauses. At some point
> it might be worth adding same, but for now I'm just going to tweak
> distribute_qual_to_rels to not believe that clauses with nonempty
> nullable_relids can be equivalence clauses.
I tried the latter, with the first patch attached below, and it fixed
the incorrect-plan problem. However, further testing showed that this
method also breaks one of the optimizations implemented in equivclass.c,
which is the ability to push constants through full joins, as in
select * from tenk1 a full join tenk1 b using (unique1)
where unique1 = 42;
We don't seem to have a regression test case for that optimization,
which is probably because it predates the availability of EXPLAIN's
COSTS OFF option. (I've added one in the second patch below.)
I thought for a minute about accepting that as fallout, but I know
for certain that we'd get pushback on it, because we did last time
I broke it :-(.
So it appears that what we have to do is upgrade the equivalence-class
machinery to handle nullable_relids properly. The second patch attached
below does that. It's considerably larger than the quick-hack patch,
though not as large as I'd originally feared.
Anyway, the question now is whether to back-patch this. We do have
pretty much the same equivalence-class infrastructure in all versions
since 8.3, so it's possible to do it ... but it's a bit nervous-making.
On the other hand, letting a known bug go unfixed isn't attractive
either.
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
disable-nullable-equivalences.patch | text/x-patch | 5.1 KB |
fix-nullable-equivalences.patch | text/x-patch | 30.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Farina | 2012-10-17 23:23:59 | Re: Deprecating RULES |
Previous Message | Simon Riggs | 2012-10-17 23:02:50 | Re: Deprecating RULES |