>> Shouldn't we check if we consumed all elements (state->next_elem >=
>> state->num_elems) inside the while loop?
>
> You're right. Fixed.
I don't think the fix is correct. arrayconst_next_fn() can still
execute state->next_elem++ without checking if we consumed all
elements. I couldn't manage to crash it though.
I am also not sure if it is proper to skip some items inside the
"next_fn", but I don't know the code to suggest a better alternative.
> + /* skip nulls if ok to do so */
> + if (state->opisstrict && state->next)
Do we need && state->next in here? It is checked for (state->next ==
NULL) 2 lines above.
> + {
> + Node *node = (Node *) lfirst(state->next);
> +
> + while (node && IsA(node, Const) && ((Const *) node)->constisnull)
Should we spell the first check like node != NULL as the rest of the code does.
> + {
> + state->next = lnext(state->next);
> + if (state->next)
> + node = (Node *) lfirst(state->next);
> + }
> + }
I think this function is also not correct as it can call
lnext(state->next) without checking.
> So far, I hadn't either. I figured one out and added it to inherit.sql.
> Basically, I needed to write the query such that an IN () expression
> doesn't get const-simplified to a Const containing array, but rather
> remains an ArrayExpr. So, arrayexpr_*() functions in predtest.c are now
> exercised.
Nice. I noticed that this part of the code was not covered at all by
the tests before.