>> 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.