Re: On disable_cost

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Peter Geoghegan <pg(at)bowt(dot)ie>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: On disable_cost
Date: 2024-10-04 14:03:12
Message-ID: CA+TgmoYObdKwLCuvFMHhRXBOCCn=HGybvQceXkZU4zug52106A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 3, 2024 at 5:52 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> It looks fine with the patch. The crux of the new logic is just
> summing up the disabled_nodes from the child nodes and checking if the
> disabled_nodes of the current node is higher than that sum. That's not
> exactly hard logic. The biggest risk seems to be not correctly
> visiting all the child nodes. I got that wrong with SubqueryScan in my
> first PoC.

Right, visiting too many or too few child nodes would be bad. The
other worry I have is about things that aren't fully Pathified --
maybe a certain node doesn't have a representation in the Path tree
but gets injected at Plan time. In that case there might be a risk of
the Disabled marker showing up in the wrong place. We do this with
sorts, for example, in the merge-join case.

> which isn't correct. Append appears disabled, but it's not. Sort is.
> Before I fixed that in the patch, I was incorrectly getting the
> "Disabled: true" under the Append. I feel we're more likely to get bug
> reports alerting us to incorrect logic when the disabled property only
> appears on disabled nodes as there are far fewer of them to look at
> and therefore it's more obvious when they're misplaced.

It's certainly possible that you're correct, and the fact that you
have this example in hand makes it more likely. I tend to gravitate
toward displaying things exactly as they exist internally because I've
had so many bad experiences with having to try to reverse-engineer the
value stored internally from whatever is printed. This problem isn't
limited to EXPLAIN, but to give one EXPLAIN-related example, we take
the row count and divide by nloops and round off to an integer and I
cannot tell you how many times that has made my life more difficult
because I really want to see planstate->instrument->ntuples, not
round(planstate->instrument->ntuples/nloops); likewise, I absolutely
loathe the fact that we round off plan->plan_rows to an integer. I
guess we do that so that we "don't confuse people," but what it means
is that I can't see the information I need to help people fix
problems, and frankly what it means is that I myself am confused. I
tend to feel like if the problem is that a user does not understand
what we are printing, that problem can be fixed by the user learning
more until they understand; but if the problem is that we don't print
enough information to understand what is truly happening inside the
data structure, there is no way out from under that problem without
recompiling, which is not where you want to be when something goes
wrong in production. Of course that idea can be taken too far. If you
refuse to translate information into a more human-understandable form
even when you can do so reliably, then you're just making life hard
for users for no benefit, and you might be right that this is such a
case. I'm not here to act like I have all the right answers. I'm just
explaining the reasoning behind what I did; and I hope that it makes
some sense to you.

--
Robert Haas
EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2024-10-04 14:20:20 Re: POC, WIP: OR-clause support for indexes
Previous Message Christoph Berg 2024-10-04 13:55:23 New PostgreSQL Contributors