Subject: Re: On disable_cost

From: Robert Haas <[email-masked]>
Date: Fri, 3 May 2024 16:33:33 -0400

On Sat, Nov 2, 2019 at 10:57 AM Tom Lane <[email-masked]> wrote:
> The idea that I've been thinking about is to not generate disabled
> Paths in the first place, thus not only fixing the problem but saving
> some cycles.  While this seems easy enough for "optional" paths,
> we have to reserve the ability to generate certain path types regardless,
> if there's no other way to implement the query.  This is a bit of a
> stumbling block :-(.  At the base relation level, we could do something
> like generating seqscan last, and only if no other path has been
> successfully generated.

Continuing my investigation into this rather old thread, I did a
rather primitive implementation of this idea, for baserels only, and
discovered that it caused a small number of planner failures running
the regression tests. Here is a slightly simplified example:

CREATE TABLE strtest (n text, t text);
CREATE INDEX strtest_n_idx ON strtest (n);
SET enable_seqscan=false;
EXPLAIN SELECT * FROM strtest s1 INNER JOIN strtest s2 ON s1.n >= s2.n;

With the patch, I get:

ERROR:  could not devise a query plan for the given query

The problem here is that it's perfectly possible to generate a valid
path for s1 -- and likewise for s2, since it's the same underlying
relation -- while respecting the enable_seqscan=false constraint.
However, all such paths are parameterized by the other of the two
relations, which means that if we do that, we can't plan the join,
because we need an unparameterized path for at least one of the two
sides in order to build a nested loop join, which is the only way to
satisfy the parameterization on the other side.

Now, you could try to fix this by deciding that planning for a baserel
hasn't really succeeded unless we got at least one *unparameterized*
path for that baserel. I haven't tried that, but I presume that if you
do, it fixes the above example, because now there will be a last-ditch
sequential scan on both sides and so this example will behave as
expected. But if you do that, then in other cases, that sequential
scan is going to get picked even when it isn't strictly necessary to
do so, just because some plan that uses it looks better on cost.
Presumably that problem can in turn be fixed by deciding that we also
need to keep disable_cost around (or the separate disable-counter idea
that we were discussing recently in another branch of this thread),
but that's arguably missing the point of this exercise.

Another idea is to remove the ERROR mentioned above from
set_cheapest() and just allow planning to continue even if some
relations end up with no paths. (This would necessitate finding and
fixing any code that could be confused by a pathless relation.) Then,
if you get to the top of the plan tree and you have no paths there,
redo the join search discarding the constraints (or maybe just some of
the constraints, e.g. allow sequential scans and nested loops, or
something). Conceptually, I like this idea a lot, but I think there
are a few problems. One is that I'm not quite sure how to find all the
code that would need to be adjusted to make it work, though the header
comment for standard_join_search() seems like it's got some helpful
tips. A second is that it's another version of the disable_cost =
infinity problem: once you find that you can't generate a path while
enforcing all of the restrictions, you just disregard the restrictions
completely, instead of discarding them only to the extent necessary. I
have a feeling that's not going to be very appealing.

Now, I suppose it might be that even if we can't remove disable_cost,
something along these lines is still worth doing, just to save CPU
cycles. You could for example try planning with only non-disabled
stuff and then do it over again with everything if that doesn't work
out, still keeping disable_cost around so that you avoid disabled
nodes where you can. But I'm kind of hoping that I'm missing something
and there's some approach that could both kill disable_cost and save
some cycles at the same time. If (any of) you have an idea, I'd love
to hear it!

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

---

From: David Rowley <[email-masked]>
Date: Sun, 5 May 2024 01:16:19 +1200

On Sat, 4 May 2024 at 08:34, Robert Haas <[email-masked]> wrote:
> something).

I don't think you'd need to wait longer than where we do set_cheapest
and find no paths to find out that there's going to be a problem.

I don't think redoing planning is going to be easy or even useful. I
mean what do you change when you replan? You can't just do
enable_seqscan and enable_nestloop as if there's no index to provide
sorted input and the plan requires some sort, then you still can't
produce a plan.  Adding enable_sort to the list does not give me much
confidence we'll never fail to produce a plan either. It just seems
impossible to know which of the disabled ones caused the RelOptInfo to
have no paths.  Also, you might end up enabling one that caused the
planner to do something different than it would do today.  For
example, a Path that today incurs 2x disable_cost vs a Path that only
receives 1x disable_cost might do something different if you just went
and enabled a bunch of enable* GUCs before replanning.

> some cycles at the same time. If (any of) you have an idea, I'd love

I think the int Path.disabledness idea is worth coding up to try it.
I imagine that a Path will incur the maximum of its subpath's
disabledness's then add_path() just needs to prefer lower-valued
disabledness Paths.

That doesn't get you the benefits of fewer CPU cycles, but where did
that come from as a motive to change this? There's no shortage of
other ways to make the planner faster if that's an issue.

David