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.