Re: [COMMITTERS] pgsql: Further fixes for degenerate outer join clauses.

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > I agree, that's a bit unfortunate, but it strikes me as pretty unlikely
> > that we're ever going to change those tests or that a code change would
> > end up causing yet another different plan before 9.1 is completely out
> > of support in the next couple years.
>
> Meh. 9.0 will be dead in a month or two, but 9.1 still has a ways to go,
> and I don't really want to be dealing with this issue every time Andreas'
> fuzz testing finds another corner case :-(. It was hard enough devising
> regression tests that used stable table data for those cases as it was.
>
> Frequently, when building a planner regression test, it's important that
> a particular plan shape be selected (eg because what you're trying to test
> is that setrefs.c postprocessing does the right thing with a given case).
> So if we take supporting brolga's behavior as-is as a requirement, it's
> not always going to be good enough to just maintain an alternate output
> file; we'd have to try to adjust the test query to make it do the right
> thing. It's bad enough dealing with 32/64bit and endianness dependencies
> for that, I don't want minute compiler codegen choices entering into it
> as well.

Certainly a good point.

> >> On the whole I'm leaning towards back-patching 33e99153e. While the case
> >> of exactly equal plan costs does come up in the regression tests (which
> >> tend to inspect plans for queries on small simple tables), I think it's
> >> relatively unlikely to happen with real-world data.
>
> > I agree it's unlikely, but I don't particularly like changing our mind
> > on a back-patching decision 3 years later to satisfy our regression
> > tests.
>
> Actually, I'd take that the other way around: now that the patch has been
> out for awhile, and we've not heard squawks that could be traced to it,
> there's a much better argument that back-patching would be safe than there
> was at the time.

I definitely agree with that when it comes to bug cases, but I'm a bit
more wary around planner changes. Still, I don't recall any specific
complaints about plan changes from pre-9.2 to 9.2+ in such corner cases
and I do agree with you that real data would make this far less likely
to happen.

> There is certainly some risk in making changes that are only for ease of
> maintenance and don't fix a provable bug ... but it's not like the other
> changes I've committed into 9.0 and 9.1 lately don't change any
> corner-case planner behaviors. Besides, you could argue that it is a bug
> if rebuilding with a different compiler can change the planner's behavior.

Good point.

Thanks!

Stephen