Robert Haas <robertmh...@gmail.com> writes:
> On Thu, Aug 3, 2023 at 5:22 AM Jian Guo <gj...@vmware.com> wrote:
>> I have write an initial patch to retire the disable_cost GUC, which labeled
>> a flag on the Path struct instead of adding up a big cost which is hard to
>> estimate. Though it involved in tons of plan changes in regression tests, I
>> have tested on some simple test cases such as eagerly generate a two-stage
>> agg plans and it worked. Could someone help to review?
> I took a look at this patch today. I believe that overall this may
> well be an approach worth pursuing. However, more work is going to be
> needed. Here are some comments:
> 1. You stated that it changes lots of plans in the regression tests,
> but you haven't provided any sort of analysis of why those plans
> changed. I'm kind of surprised that there would be "tons" of plan
> changes. You (or someone) should look into why that's happening.
I've not read the patch, but given this description I would expect
there to be *zero* regression changes --- I don't think we have any
test cases that depend on disable_cost being finite. If there's more
than zero changes, that very likely indicates a bug in the patch.
Even if there are places where the output legitimately changes, you
need to justify each one and make sure that you didn't invalidate the
intent of that test case.
BTW, having written that paragraph, I wonder if we couldn't get
the same end result with a nearly one-line change that consists of
making disable_cost be IEEE infinity. Years ago we didn't want
to rely on IEEE float semantics in this area, but nowadays I don't
see why we shouldn't.
regards, tom lane
