Hi,
On Sun, 2023-07-16 at 22:36 +0200, Tomas Vondra wrote:
> This kept bothering me, so I looked at it today, and reworked it to
> use
> the IOS approach.
Initial comments on patch 20230716:
* check_index_filter() alredy looks at "canreturn", which should mean
that you don't need to later check for opcintype<>opckeytype. But
there's a comment in IndexNext() indicating that's a problem -- under
what conditions is it a problem?
* (may be a matter of taste) Recomputing the bitmapset from the
canreturn array in check_index_filter() for each call seems awkward. I
would just iterate through the bitmapset and check that all are set
true in the amcanreturn array.
* There are some tiny functions that don't seem to add much value or
have slightly weird APIs. For instance, match_filter_to_index() could
probably just return a boolean, and maybe doesn't even need to exist
because it's such a thin wrapper over check_index_filter(). Similarly
for fix_indexfilter_clause(). I'm OK with tiny functions even if the
only value is a comment, but I didn't find these particularly helpful.
* fix_indexfilter_references() could use a better comment. Perhaps
refactor so that you can share code with fix_indexqual_references()?
* it looks like index filters are duplicated with ordinary filters, is
there a reason for that?
* I'm confused about the relationship of an IOS to an index filter. It
seems like the index filter only works for an ordinary index scan? Why
is that?
Regards,
Jeff Davis