Re: ERROR: wrong varnullingrels (b 3) (expected (b)) for Var 2/1

Richard Guo <guofenglinux(at)gmail(dot)com> writes:
> On Wed, May 31, 2023 at 10:47 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
>> When we transform the first form of identity 3 to the second form, we've
>> converted Pb*c to Pbc in deconstruct_distribute_oj_quals. But we
>> neglect to consider that rel C might be a RTE_SUBQUERY and contains
>> quals that have lateral references to B. So the B vars in such quals
>> have wrong nulling bitmaps and we'd finally notice that when we do
>> fix_upper_expr for the NestLoopParam expressions.

Right. One question that immediately raises is whether it's even safe
to apply the identity if C has lateral references to B, because that
almost certainly means that C will produce different output when
joined to a nulled B row than when joined to a not-nulled row.
I think that's okay because if the B row will fail Pab then it doesn't
matter what row(s) C produces, but maybe I've missed something.

> We can identify in which form of identity 3 the plan is built up by
> checking the relids of the B/C join's outer rel. If it's in the first
> form, the outer rel's relids must contain the A/B join. Otherwise it
> should only contain B's relid. So I'm considering that maybe we can
> adjust the nulling bitmap for nestloop parameters according to that.
> Attached is a patch for that. Does this make sense?

Hmm. I don't really want to do it in identify_current_nestloop_params
because that gets applied to all nestloop params, so it seems like
that risks masking bugs of other kinds. I'd rather do it in
process_subquery_nestloop_params, which we know is only applied to
subquery LATERAL references. So more or less as attached.

(I dropped the equal() assertions in process_subquery_nestloop_params
because they've never caught anything and it'd be too complicated to
make them still work.)

Thoughts?

regards, tom lane