(2019/02/14 18:44), Etsuro Fujita wrote:
(2019/02/12 20:43), Antonin Houska wrote:
Etsuro Fujita<fujita.ets...@lab.ntt.co.jp> wrote:
Here are my review comments:
root->upper_targets[UPPERREL_FINAL] = final_target;
+ root->upper_targets[UPPERREL_ORDERED] = final_target;
root->upper_targets[UPPERREL_WINDOW] = sort_input_target;
root->upper_targets[UPPERREL_GROUP_AGG] = grouping_target;
I think that's a good idea. I think we'll soon need the PathTarget for
UPPERREL_DISTINCT, so how about saving that as well like this?
root->upper_targets[UPPERREL_FINAL] = final_target;
+ root->upper_targets[UPPERREL_ORDERED] = final_target;
+ root->upper_targets[UPPERREL_DISTINCT] = sort_input_target;
root->upper_targets[UPPERREL_WINDOW] = sort_input_target;
root->upper_targets[UPPERREL_GROUP_AGG] = grouping_target;
I see nothing wrong about this.
Cool!
Another is about this:
/*
+ * Set reltarget so that it's consistent with the paths. Also it's more
+ * convenient for FDW to find the target here.
+ */
+ ordered_rel->reltarget = target;
I think this is correct if there are no SRFs in the targetlist, but
otherwise
I'm not sure this is really correct because the relation's reltarget
would not
match the target of paths added to the relation after
adjust_paths_for_srfs().
I wouldn't expect problem here. create_grouping_paths() also sets the
reltarget although adjust_paths_for_srfs() can be alled on grouped_rel
afterwards.
Yeah, but as I said in a previous email, I think the root->upper_targets
change would be enough at least currently for FDWs to consider
performing post-UPPERREL_GROUP_AGG steps remotely, as shown in my
patches, so I'm inclined to leave the retarget change for future work.
So, here is an updated patch. If there are no objections from you or
anyone else, I'll commit the patch as a preliminary one for what's
proposed in this thread.
Best regards,
Etsuro Fujita
>From 5ade52b576cb8403d3a079fecf4c938225cc3bd7 Mon Sep 17 00:00:00 2001
From: Etsuro Fujita <efuj...@postgresql.org>
Date: Fri, 15 Feb 2019 21:13:36 +0900
Subject: [PATCH] Save PathTargets for distinct/ordered relations in
root->upper_targets[]
Previously, we only saved the PathTargets for grouping, window and final
relations in root->upper_targets[] in grouping_planner. To improve the
convenience of extensions, save the PathTargets for distinct and ordered
relations as well.
Author: Antonin Houska, with an additional change by me
Discussion: https://postgr.es/m/10994.1549559088@localhost
---
src/backend/optimizer/plan/planner.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index ddb86bd0c3..a4ec4456e5 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -2035,6 +2035,8 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
* of the corresponding upperrels might not be needed for this query.
*/
root->upper_targets[UPPERREL_FINAL] = final_target;
+ root->upper_targets[UPPERREL_ORDERED] = final_target;
+ root->upper_targets[UPPERREL_DISTINCT] = sort_input_target;
root->upper_targets[UPPERREL_WINDOW] = sort_input_target;
root->upper_targets[UPPERREL_GROUP_AGG] = grouping_target;
--
2.19.2
