On Wed, 16 Apr 2025 at 10:09, Ilia Evdokimov <ilya.evdoki...@tantorlabs.com> wrote: > I've prepared the updated patches as discussed, including the addition > of estimated lookups in the EXPLAIN output for Memoize. Please let me > know if you have any feedback or further suggestions.
While this is fresh, as I might forget before the July CF... I know I did suggest that the hit_ratio should be a percent and it should be multiplied by 100.0. This seems fine for the text format as you can have the % unit there. However, looking at ExplainPropertyFloat(), we don't print the unit for non-text formats. I wonder if printing a number between 0 and 100 there will be confusing. I don't believe we have anything else in EXPLAIN that shows a percentage. I don't quite know what to do about this. One thought I had was to only * 100 for text format, but that might be more confusing. Aside from that, I'd prefer the new fields in struct Memoize to be prefixed with "est_" the same as the existing "est_entries" field. I'm unsure why MemoizePath.calls becomes Memoize.lookups. Seems unnecessary and just means more brain space being used to maintain a mental model. David
