On Fri, Feb 2, 2024 at 7:36 PM David Rowley <dgrowle...@gmail.com> wrote:
> Now for the other stuff you had. I didn't really like this part: > > + /* > + * Set target for partial_distinct_rel as generate_useful_gather_paths > + * requires that the input rel has a valid reltarget. > + */ > + partial_distinct_rel->reltarget = cheapest_partial_path->pathtarget; > > I think we should just make it work the same way as > create_grouping_paths(), where grouping_target is passed as a > parameter. > > I've done it that way in the attached. The change looks good to me. BTW, I kind of doubt that 'create_partial_distinct_paths' is a proper function name given what it actually does. It not only generates distinct paths based on input_rel's partial paths, but also adds Gather/GatherMerge on top of these partially distinct paths, followed by a final unique/aggregate path to ensure uniqueness of the final result. So maybe 'create_parallel_distinct_paths' or something like that would be better? I asked because I noticed that in create_partial_grouping_paths(), we only generate partially aggregated paths, and any subsequent FinalizeAggregate step is called in the caller. Thanks Richard
