Re: BUG #18806: When enable_rartitionwise_join is set to ON, the database shuts down abnormally

Hi Richard,

On Wed, Feb 12, 2025 at 11:30 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> On Wed, Feb 12, 2025 at 10:04 PM Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)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;
}

That is, should we copy relids not only when translation is needed but
also in the general case? Would that be a bigger band-aid than
necessary?

--
Thanks, Amit Langote