Hello David,
Thank you very much for updating the patches and clearly addressing
these points.
On Mon, Apr 7, 2025 at 8:04 AM David Rowley <dgrowleyml@gmail.com> wrote:
>
> I used the attached .txt file to highlight the places where the
> iterator returned the same member twice and saw only that
> find_ec_member_matching_expr() does this.
>
> Because during createplan, we'll have the specific RelOptInfo that we
> need the EquivalenceMember for, we'll be passing the "relids" of that
> RelOptInfo to setup_eclass_member_iterator(), in which case, I think
> it's fine to store the member in just one of the ec_childmembers[]
> array slots for just one of the relids making up the
> RELOPT_OTHER_JOINREL's component relids as that means it'll be found
> once only due to how eclass_member_iterator_next() looks at all of the
> ec_childmembers[] elements for the given relids.
>
> Doing this also allows the code in add_child_eq_member() to be simplified.
I agree with you. Using the first member of relids as the array index
is simple, clear, and effective, significantly simplifying the logic
and implementation.
> I made this happen in the attached v41 patch, and that's the last
> outstanding issue that I had for this.
>
> I think this is worthy of getting into v18. Does anyone else think
> differently? It'd be good to know that soon.
Thank you very much for sharing this new patch. It looks good to me,
and I'd also be very happy to see this committed in v18.
--
Best regards,
Yuya Watari
