On Fri, Mar 1, 2019 at 5:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> James Coleman <jtc331@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