Thanks for checking.
On Thu, Oct 31, 2019 at 1:09 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Also, the existing logic around eclass_indexes is that it's only
> set for baserels and we know it is valid after we've finished
> EC merging. I don't much like modifying add_child_rel_equivalences
> to have some different opinions about that for joinrels.
>
> It'd probably be better to just eat the cost of doing
> get_common_eclass_indexes over again when it's time to do
> add_child_rel_equivalences for a joinrel, since we aren't (I hope)
> going to do that more than once per joinrel anyway.
If you mean once per child joinrel, then yes. Implemented this
approach in the attached updated patch.
ISTM, get_common_eclass_indexes() returns the same value for all the
child joinrels (or really for its outer and inner component rels) as
it does for the parent joinrel. So, it might be better to set
eclass_indexes in parent joinrel once and use the same value for all
its descendant joinrels. Although, we'd need to export
get_common_eclass_indexes() out of equivclass.c to call it from
build_join_rel() such that it doesn't require messing with where the
joinrel is added to the global data structure. Maybe that complicates
eclass_indexes infrastructure though.
> This would
> probably require refactoring things so that there are separate
> entry points to add child equivalences for base rels and join rels.
> But that seems cleaner anyway than what you've got here.
Separate entry points sounds better, but only in HEAD? Should we have
separate entry points in PG 12 too?
Attached updated patch only for HEAD.
Thanks,
Amit