[ back from PGCon ... ] "Anton A. Melnikov" <aamelni...@inbox.ru> writes: > On 04.05.2023 15:22, Tom Lane wrote: >> Under what circumstances would you be trying to inject INDEX_VAR >> into a nullingrel set? Only outer-join relids should ever appear there.
> The thing is that i don't try to push INDEX_VAR into a nullingrel set at all, > i just try to replace the existing rt_index that equals to INDEX_VAR in Var > nodes with > the defined positive indexes by using ChangeVarNodes_walker() function call. Hmm. That implies that you're changing plan data structures around after setrefs.c, which doesn't seem like a great design to me --- IMO that ought to happen in PlanCustomPath, which will still see the original varnos. However, it's probably not worth breaking existing code for this, so now I agree that ChangeVarNodes ought to (continue to) allow negative rt_index. > Therefore it also seems better and more logical to me in the case of an index > that > cannot possibly be a member of the Bitmapset, immediately return false. > Here is a patch like that. I do not like the blast radius of this patch. Yes, I know about that comment in bms_is_member --- I wrote it, if memory serves. But it's stood like that for more than two decades, and I believe it's caught its share of mistakes. This issue doesn't seem like a sufficient reason to change a globally-visible behavior. I think the right thing here is not either of your patches, but to tweak adjust_relid_set() to not fail on negative oldrelid. I'll go make it so. regards, tom lane