On Tue, 20 Jul 2021 at 19:28, Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
> > The new code in statext_is_compatible_clause_internal() is a little
> > hard to follow because some of the comments aren't right
>
> I ended up doing something slightly different - examine_opclause_args
> now "returns" a list of expressions, instead of explicitly setting two
> parameters. That means we can do a simple foreach() here, which seems
> cleaner. It means we have to extract the expressions from the list in a
> couple places, but that seems acceptable. Do you agree?
Yes, that looks much neater.
> > In mcv_get_match_bitmap(), perhaps do the RESULT_IS_FINAL() checks
> > first in each loop.
>
> This is how master already does that now, and I wonder if it's done in
> this order intentionally. It's not clear to me doing it in the other way
> would be faster?
Ah OK, it just felt more natural to do it the other way round. I
suppose though, that for the first clause, the is-final check isn't
going to catch anything, whereas the is-null checks might. For the
remaining clauses, it will depend on the data as to which way is
faster, but it probably isn't going to make any noticeable difference
either way. So, although it initially seems a bit counter-intuitive,
it's probably better the way it is.
> I guess the last thing is maybe mentioning this in
> the docs, adding an example etc.
Yeah, good idea.
Regards,
Dean