James Coleman <jtc...@gmail.com> writes: > [ saop_is_not_null-v10.patch ]
I went through this again, and this time (after some more rewriting of the comments) I satisfied myself that the logic is correct. Hence, pushed. I stripped down the regression test cases somewhat. Those were good for development, but they were about doubling the runtime of test_predtest.sql, which seemed excessive for testing one feature. I also tweaked it to recognize the case where we can prove the array, rather than the scalar, to be null. I'm not sure how useful that is in practice, but it seemed weird that we'd exploit that only if we can also prove the scalar to be null. > I've implemented this fix as suggested. The added argument I've > initially called "allow_false"; I don't love the name, but it is > directly what it does. I'd appreciate other suggestions (if you prefer > it change). I was tempted to rename it to "weak", but decided that that might be overloading that term too much in this module. Left it as-is. > Ideally I think this case would be handled elsewhere in the optimizer > by directly replacing the saop and a constant NULL array with NULL, > but this isn't the patch to do that, and at any rate I'd have to do a > bit more investigation to understand how to do such (if it's easily > possible). Take a look at the ScalarArrayOp case in eval_const_expressions. Right now it's only smart about the all-inputs-constant case. I'm not really convinced it's worth spending cycles on the constant- null-array case, but that'd be where to do it if we want to do it in a general way. (I think that what I added to clause_is_strict_for is far cheaper, because while it's the same test, it's in a place that we won't hit during most queries.) > I also updated the tests with a new helper function "opaque_array" to > make it easy to test cases where the planner can't inspect the array. Yeah, that's a win. I used that in most of the tests that I left in place, but I kept a couple with long arrays just so we'd have code coverage of the parts of clause_is_strict_for that need to detect array size. regards, tom lane