Re: On disable_cost

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Greg Sabino Mullane <htamfids(at)gmail(dot)com>, Zhenghua Lyu <zlyu(at)vmware(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: On disable_cost
Date: 2024-04-03 19:21:24
Message-ID: CA+TgmoZEg1tyW31t3jxhvDwff29K=2C9r6722SuFb=3XVKWkow@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Apr 2, 2024 at 12:58 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Not really. But if you had, say, a join of a promoted path to a
> disabled path, that would be treated as on-par with a join of two
> regular paths, which seems like it'd lead to odd choices. Maybe
> it'd be fine, but my gut says it'd likely not act nicely. As you
> say, it's a lot easier to believe that only-promoted or only-disabled
> situations would behave sanely.

Makes sense.

I wanted to further explore the idea of just not generating plans of
types that are currently disabled. I looked into doing this for
enable_indexscan and enable_indexonlyscan. As a first step, I
investigated how those settings work now, and was horrified. I don't
know whether I just wasn't paying attention back when the original
index-only scan work was done -- I remember discussing
enable_indexonlyscan with you at the time -- or whether it got changed
subsequently. Anyway, the current behavior is:

[A] enable_indexscan=false adds disable_cost to the cost of every
Index Scan path *and also* every Index-Only Scan path. So disabling
index-scans also in effect discourages the use of index-only scans,
which would make sense if we didn't have a separate setting called
enable_indexonlyscan, but we do. Given that, I think this is
completely and utterly wrong.

[b] enable_indexonlyscan=false causes index-only scan paths not to be
generated at all, but instead, we generate index-scan paths to do the
same thing that we would not have generated otherwise. This is weird
because it means that disabling one plan type causes us to consider
additional plans of another type, which seems like a thing that a user
might not expect. It's more defensible than [A], though, because you
could argue that we only omit the index scan path as an optimization,
on the theory that it will always lose to the index-only scan path,
and thus if the index-only scan path is not generated, there's a point
to generating the index scan path after all, so we should. However, it
seems unlikely to me that someone reading the one line of
documentation that we have about this parameter would be able to guess
that it works this way.

Here's an example of how the current system behaves:

robert.haas=# explain select count(*) from pgbench_accounts;
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------
Aggregate (cost=2854.29..2854.30 rows=1 width=8)
-> Index Only Scan using pgbench_accounts_pkey on pgbench_accounts
(cost=0.29..2604.29 rows=100000 width=0)
(2 rows)

robert.haas=# set enable_indexscan=false;
SET
robert.haas=# explain select count(*) from pgbench_accounts;
QUERY PLAN
------------------------------------------------------------------------------
Aggregate (cost=2890.00..2890.01 rows=1 width=8)
-> Seq Scan on pgbench_accounts (cost=0.00..2640.00 rows=100000 width=0)
(2 rows)

robert.haas=# set enable_seqscan=false;
SET
robert.haas=# set enable_bitmapscan=false;
SET
robert.haas=# explain select count(*) from pgbench_accounts;
QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------
Aggregate (cost=10000002854.29..10000002854.30 rows=1 width=8)
-> Index Only Scan using pgbench_accounts_pkey on pgbench_accounts
(cost=10000000000.29..10000002604.29 rows=100000 width=0)
(2 rows)

robert.haas=# set enable_indexonlyscan=false;
SET
robert.haas=# explain select count(*) from pgbench_accounts;
QUERY PLAN
-----------------------------------------------------------------------------------------------
Aggregate (cost=10000002890.00..10000002890.01 rows=1 width=8)
-> Seq Scan on pgbench_accounts
(cost=10000000000.00..10000002640.00 rows=100000 width=0)
(2 rows)

The first time we run the query, it picks an index-only scan because
it's the cheapest. When index scans are disabled, the query now picks
a sequential scan, even though it wasn't use an index-only scan,
because the index scan that it was using is perceived to have become
very expensive. When we then shut off sequential scans and bitmap-only
scans, it switches back to an index-only scan, because setting
enable_indexscan=false didn't completely disable index-only scans, but
just made them expensive. But now everything looks expensive, so we go
back to the same plan we had initially, except with the cost increased
by a bazillion. Finally, when we disable index-only scans, that
removes that plan from the pool, so now we pick the second-cheapest
plan overall, which in this case is a sequential scan.

So just to see what would happen, I wrote a patch to make
enable_indexscan and enable_indexonlyscan do exactly what they say on
the tin: when you set one of them to false, paths of that type are not
generated, and nothing else changes. I found that there are a
surprisingly large number of regression tests that rely on the current
behavior, so I took a crack at fixing them to achieve their goals (or
what I believed their goals to be) in other ways. The resulting patch
is attached for your (or anyone's) possible edification.

Just to be clear, I have no immediate plans to press forward with
trying to get something committed here. It seems pretty clear to me
that we should fix [A] in some way, but maybe not in the way I did it
here. It's also pretty clear to me that the fact that enable_indexscan
and enable_indexonlyscan work completely differently from each other
is surprising at best, wrong at worst, but here again, what this patch
does about that is not above reproach. I think it may make sense to
dig through the behavior of some of the remaining enable_* GUCs before
settling on a final strategy here, but I thought that the finds above
were interesting enough and bizarre enough that it made sense to drop
an email now and see what people think of all this before going
further.

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

Attachment Content-Type Size
v1-0001-Rationalize-the-behavior-of-enable_indexscan-and-.patch application/octet-stream 48.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nazir Bilal Yavuz 2024-04-03 19:25:01 Re: Use streaming read API in ANALYZE
Previous Message Daniel Gustafsson 2024-04-03 19:12:47 Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?