On Wed, Feb 12, 2025 at 11:30 PM Richard Guo <guofenglinux@gmail.com> wrote: > On Wed, Feb 12, 2025 at 10:04 PM Ashutosh Bapat > <ashutosh.bapat.oss@gmail.com> wrote: > > Hmm. The code there assumes that all the Relids will at least have one > > parent each of the children involved. For some reason > > sjinfo->min_lefthand has only relid 1 but not 2 or 5. 2 and 5 are > > actually the parent relids of the children passed in respectively. The > > join is between 2 and 5, then why is 1 appearing in the min_lefthand. > > It might be legitimate, but we need to find the reason. If it's > > legitimate, I think we need to copy the Relids which haven't undergone > > any translation so as to keep them isolated from the parent relids. > > Yes, it's legitimate. For the semijoin, its join clause only > references {1} and {5}, with no other ordering restrictions. > Therefore, the minimum LHS for this join consists only of {1}. > > Instead of copying the untranslated Relids and freeing them later, I > think it might be better to modify free_child_join_sjinfo() to avoid > freeing the untranslated members of child_sjinfo.
free_child_join_sjinfo() frees min_lefthand and other fields initialized by build_child_join_sjinfo(), assuming that adjust_child_relids() creates a copy. However, this does not always seem to be the case, as demonstrated in this report.
I'm wondering if the following line in adjust_child_relids() should be using bms_copy() instead:
/* Otherwise, return the original set without modification. */ return relids; }
This fix is ok for me. Keeping the child's info isolated from the parents seems safer.