Re: Pathify RHS unique-ification for semijoin planning

On Thu, 22 May 2025 at 17:28, Andy Fan <zhihuifan1213(at)163(dot)com> wrote:

> Richard Guo <guofenglinux(at)gmail(dot)com> writes:
>
> Hi,
>
> > However, in the case of sort-based implementation,
> > this function pays no attention to the subpath's pathkeys or the
> > pathkeys of the resulting output.
>
> Good finding!
>
> >
> > In addition to this specific issue, it seems to me that there are
> > other potential issues in create_unique_path().
> >
> > * Currently, we only consider the cheapest_total_path of the RHS when
> > unique-ifying it.
>
> I think it is better have a check the tuple_fraction for the startup_cost
> factor, for some paths where the total cost is high but the required
> fraction is lower.
>
> > I think this may cause us to miss some optimization
> > opportunities. For example, there might be a path with a better sort
> > order that isn't the cheapest-total one. Such a path could help avoid
> > a sort at a higher level, potentially resulting in a cheaper overall
> > plan.
>
> > * In create_unique_path(), we currently rely on heuristics to decide
> > whether to use a hash-based or sort-based method. I think a better
> > approach would be to create paths for both methods and let add_path()
> > determine which one is better, similar to how we handle path selection
> > in other parts of the planner.
> >
> > Therefore, I'm thinking that maybe we could create a new RelOptInfo
> > for the RHS rel to represent its unique-ified version, and then
> > generate all worthwhile paths for it,
>
> This sounds great for me. and I think we can keep the fraction
> cheapest path on the new RelOptInfo as well, then all the things should
> be on the way.
>
> > To be concrete, I'm envisioning something like the following:
> >
> > if (bms_equal(sjinfo->syn_righthand, rel2->relids) &&
> > - create_unique_path(root, rel2, rel2->cheapest_total_path,
> > - sjinfo) != NULL)
> > + (rel2_unique = create_unique_rel(root, rel2, sjinfo)) !=
> NULL)
> >
> > ...
> >
> > - add_paths_to_joinrel(root, joinrel, rel1, rel2,
> > - JOIN_UNIQUE_INNER, sjinfo,
> > + add_paths_to_joinrel(root, joinrel, rel1, rel2_unique,
> > + JOIN_INNER, sjinfo,
> > restrictlist);
> > - add_paths_to_joinrel(root, joinrel, rel2, rel1,
> > - JOIN_UNIQUE_OUTER, sjinfo,
> > + add_paths_to_joinrel(root, joinrel, rel2_unique, rel1,
> > + JOIN_INNER, sjinfo,
> > restrictlist);
> >
> > In addition, by changing the join from "rel1" and "rel2" using
> > JOIN_UNIQUE_OUTER or JOIN_UNIQUE_INNER to a join between "rel1" and
> > "rel2_unique" using a standard JOIN_INNER, we might be able to get
> > rid of the JOIN_UNIQUE_OUTER and JOIN_UNIQUE_INNER jointypes.
>
> >>if we can, +10.

Agree
Pls kindly release a path for this？

>
>

>
>
>
>