On Fri, Mar 1, 2019 at 5:28 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > 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.
Thanks! > 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. Just for my own understanding: I thought the "if (arrayconst->constisnull)" took care of the array constant being null? I don't see a check on the scalar node / lhs. I do see you added a check for the entire clause being null, but I'm not sure I understand when that would occur (unless it's only in the recursive case?) > 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.) Thanks for the pointer; I'll take a look if for no other reason than curiosity. Thanks, James Coleman
