On Tue, Jan 15, 2019 at 11:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Well, as I said upthread, it seems like we need to think a bit more
> carefully about what it is that clause_is_strict_for is testing ---
> and if that ends up finding that some other name is more apposite,
> I'd not have any hesitation about renaming it. But we're really
> missing a bet if the ScalarArrayOp-specific check isn't inside that
> recursive check of an "atomic" expression's properties. The
> routines above there only recurse through things that act like
> AND or OR, but clause_is_strict_for is capable of descending
> through other stuff as long as it's strict in some sense. What
> we need to be clear about here is exactly what that sense is.
All right, I'll look over all of the other callers and see what makes
sense as far as naming (or perhaps consider a new parallel function;
unsure at this point.)
I might also try to see if we can edit the tests slightly to require
the recursion case to be exercised.
One other question on testing: do you think the "calculated array"
tests are good enough by themselves (i.e., remove the ones with array
constants of > 100 values)? I dislike that it's not as obvious what's
going on, but given that the code shouldn't care about array size
anyway...so if there's an inclination to fewer tests that's the first
place I'd look at cutting them.
James Coleman
