Re: [HACKERS] Partition-wise aggregation/grouping

I have reviewed and tested the patch and here are my couple of points:

     /*
-     * If the input rel belongs to a single FDW, so does the grouped rel.
+     * If the input rel belongs to a single FDW, so does the grouped rel. Same
+     * for the partially_grouped_rel.
      */
     grouped_rel->serverid = input_rel->serverid;
     grouped_rel->userid = input_rel->userid;
     grouped_rel->useridiscurrent = input_rel->useridiscurrent;
     grouped_rel->fdwroutine = input_rel->fdwroutine;
+    partially_grouped_rel->serverid = input_rel->serverid;
+    partially_grouped_rel->userid = input_rel->userid;
+    partially_grouped_rel->useridiscurrent = input_rel->useridiscurrent;
+    partially_grouped_rel->fdwroutine = input_rel->fdwroutine;

In my earlier mail where I have posted a patch for this partially grouped rel changes, I forgot to put my question on this.
I was unclear about above changes and thus passed grouped_rel whenever we wanted to work on partially_grouped_rel to fetch relevant details.

One idea I thought about is to memcpy the struct once we have set all required fields for grouped_rel so that we don't have to do similar stuff for partially_grouped_rel.

---

+             * Insert a Sort node, if required.  But there's no point in
+             * sorting anything but the cheapest path.
              */
-            if (root->group_pathkeys)
+            if (!pathkeys_contained_in(root->group_pathkeys, path->pathkeys))
+            {
+                if (path != linitial(partially_grouped_rel->pathlist))
+                    continue;

Paths in pathlist are added by add_path(). Though we have paths is pathlist is sorted with the cheapest total path, we generally use RelOptInfo->cheapest_total_path instead of using first entry, unlike partial paths. But here you use the first entry like partial paths case. Will it better to use cheapest total path from partially_grouped_rel? This will require calling set_cheapest on partially_grouped_rel before we call this function.

Attached top-up patch doing this along with few indentation fixes.

Rest of the changes look good to me.

Once this gets in, I will re-base my other patches accordingly.

And, thanks for committing 0006.

Thanks