On Sat, Mar 2, 2019 at 5:29 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> James Coleman <jtc331@gmail.com> writes:
> > On Fri, Mar 1, 2019 at 5:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> 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?
>
> Yeah, but only after we've already matched the subexpr to the LHS,
> otherwise we'd not be bothering to determine the array's size.
>
> On the other hand, if for some reason we know that the array side
> is NULL, we can conclude that the ScalarArrayOp returns NULL
> independently of what the LHS is.
Thanks for the detailed explanation.
> > 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?)
>
> Yeah, it's to handle the case where we run into a constant NULL
> below a strict node. Typically, that doesn't happen because
> eval_const_expressions would have const-folded the upper node, but
> it's not impossible given all the cases that clause_is_strict_for
> can recurse through now. (The coverage bot does show that code being
> reached, although perhaps that only occurs for null-below-ScalarArrayOp,
> in which case it might be dead if we were to make eval_const_expressions
> smarter.)
Ah, that makes sense.
James Coleman
