Re: On disable_cost
Jonathan S. Katz <jkatz@postgresql.org>
From: "Jonathan S. Katz" <jkatz@postgresql.org>
To: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, David Rowley
<dgrowleyml@gmail.com>, Peter Geoghegan <pg@bowt.ie>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, andrew@ankane.org
Date: 2024-08-23T21:33:12Z
Lists: pgsql-hackers
On 8/23/24 3:32 PM, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> I find both of your proposed solutions above to be pretty inelegant, > > They are that. If we were working in a green field I'd not propose > such things ... but we aren't. I believe there are now a fair number > of out-of-core index AMs, so I'd rather not break all of them if we > don't have to. For distribution of index AMs in the wild, it's certainly > 1 now, and increasing. They're not the easiest extension types to build out, so it's not as widely distributed as some of the other APIs, but there are a bunch out there, as well as language-specific libs (e.g. pgrx for Rust) that offer wrappers around them. >> and I think if this problem occurred with a core AM, I'd push for an >> API break rather than accept the ugliness. "This path is not valid >> because the AM cannot support it", "this path is crazy expensive", and >> "the user told us not to do it this way" are three different things, >> and signalling two or more of them in the same way muddies the water >> in a way that I don't like. > > I think it's not that bad, because we can limit the knowledge of this > hack to the amcostestimate interface, which doesn't really deal in > "the user told us not to do it this way" at all. That argues against > my first proposal though (having amcostestimate touch disabled_nodes > directly). I now think that a reasonable compromise is to say that > setting indexTotalCost to +Inf signals that "the AM cannot support > it". That's not conflated too much with the other case, since even a > crazy-expensive cost estimate surely ought to be finite. We can have > cost_index untangle that case into a separate failure return so that > the within-the-core-optimizer APIs remain clean. > > While that would require hnsw to make a small code change (return > +Inf not DBL_MAX), that coding should work in back branches too, > so they don't even need a version check. +1 for this approach (I'll do a quick test in my pgvector workspace just to ensure it gets the same results in the older version). Jonathan
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