Re: On disable_cost
David Rowley <dgrowleyml@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Robert Haas <robertmhaas@gmail.com>,
Peter Geoghegan <pg@bowt.ie>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-07-04T01:29:39Z
Lists: pgsql-hackers
On Wed, 3 Jul 2024 at 09:49, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Heikki Linnakangas <hlinnaka@iki.fi> writes: > > 3. Oh right, bitmap scan, I forgot about that one. Let's disable that too: > > Yeah, I've hit that too, although more often (for me) it's the first > choice of plan. In any case, it usually takes more than one change > to get to a seqscan. I commonly hit this too. I think the current behaviour is born out of the fact that we don't produce both an Index Scan and an Index Only Scan for the same index. We'll just make the IndexPath an index only scan, if possible based on: index_only_scan = (scantype != ST_BITMAPSCAN && check_index_only(rel, index)); The same isn't true for Bitmap Index Scans. We'll create both IndexPaths and BitmapHeapPaths and let them battle it out in add_path(). I suspect this is why it's been coded that enable_indexscan also disables Index Only Scans. Now, of course, it could work another way, but I also still think that doing so is changing well-established behaviour that I don't recall anyone ever complaining about besides Robert. Robert's complaint seems to have originated from something he noticed while hacking on code rather than actually using the database for something. I think the argument for changing it should have less weight due to that. I understand that we do have inconsistencies around this stuff. For example, enable_sort has no influence on Incremental Sorts like enable_indexscan has over Index Only Scan. That might come from the fact that we used to, up until a couple of releases ago, produce both sort path types and let them compete in add_path(). That's no longer the case, we now just do incremental sort when we can, just like we do Index Only Scans when we can. Despite those inconsistencies, I wouldn't vote for changing either of them to align with the other. It just feels too long-established behaviour to be messing with. I feel it might be best to move this patch to the back of the series or just drop it for now as it seems to be holding up the other stuff from moving forward, and that stuff looks useful and worth changing. David
Commits
-
Doc: add detail about EXPLAIN's "Disabled" property
- 84b8fccbe5c2 18.0 landed
-
Adjust EXPLAIN's output for disabled nodes
- 161320b4b960 18.0 landed
-
Fix order of parameters in a cost_sort call
- 87b6c3c0b703 18.0 landed
-
Show number of disabled nodes in EXPLAIN ANALYZE output.
- c01743aa4866 18.0 landed
-
Treat number of disabled nodes in a path as a separate cost metric.
- e22253467942 18.0 landed
-
Remove grotty use of disable_cost for TID scan plans.
- e4326fbc60c4 18.0 landed