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

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> Stephen Frost <sfrost(at)snowman(dot)net> writes:
>>> Have an alternate file for those other cases, rather than remove the
>>> test? The complaint was about one buildfarm member, so hopefully that's
>>> practical and doesn't require a lot of different permutations.

>> I considered that but don't find it practical or attractive, especially
>> not if the only way to keep such a file updated is to wait and see whether
>> the buildfarm complains.

> 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.

>> 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.

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.

regards, tom lane