Re: On disable_cost

On 8/23/24 3:32 PM, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)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