Re: ERROR: wrong varnullingrels (b 3) (expected (b)) for Var 2/1
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
Cc: Markus Winand <markus.winand@winand.at>, pgsql-hackers@postgresql.org
Date: 2023-06-19T19:37:24Z
Lists: pgsql-hackers
Attachments
- 0001-partial-revert-bfd332b3f-63e4f13d.patch (text/x-diff) patch 0001
- 0002-fix-it-in-identify_current_nestloop_params.patch (text/x-diff) patch 0002
- 0003-fix-lateral_relids.patch (text/x-diff) patch 0003
I wrote: > Richard Guo <guofenglinux@gmail.com> writes: >> So it seems that we need to do nullingrel adjustments in a more common >> place. > I agree: this suggests that we fixed it in the wrong place. So pursuant to that, 0001 attached reverts the code changes from bfd332b3f and 63e4f13d2 (keeping the test cases and some unrelated comment fixes). Then the question is what to do instead. I've not come up with a better idea than to hack it in identify_current_nestloop_params (per 0002), as you proposed upthread. I don't like this too much, as it's on the hairy edge of making setrefs.c's nullingrel cross-checks completely useless for NestLoopParams; but the alternatives aren't attractive either. >> I think it exposes a new issue. It seems that we extract a problematic >> lateral_relids from lateral references within PlaceHolderVars in >> create_lateral_join_info. I doubt that we should use ph_lateral >> directly. It seems more reasonable to me that we strip outer-join >> relids from ph_lateral and then use that for lateral_relids. I experimented with that (0003) and it fixes your example query. I think it is functionally okay, because the lateral_relids just need to be a sufficient subset of the lateral references' requirements to ensure we can evaluate them where needed; other mechanisms should ensure that the right sorts of joins happen. It seems a bit unsatisfying though, especially given that we just largely lobotomized setrefs.c's cross-checks for these same references. I don't have a better idea however, and beta2 is fast approaching. regards, tom lane
Commits
-
Don't include outer join relids in lateral_relids bitmapsets.
- efeb12ef0bfe 16.0 landed
-
Centralize fixups for mismatched nullingrels in nestloop params.
- 0655c03ef9cc 16.0 landed
-
Fix "wrong varnullingrels" for Memoize's lateral references, too.
- 63e4f13d2a78 16.0 landed
-
Fix "wrong varnullingrels" for subquery nestloop parameters.
- bfd332b3fda5 16.0 landed