Etsuro Fujita escribió:
> > From: Hitoshi Harada [mailto:umi.tan...@gmail.com]
>
> > I tried several ways but I couldn't find big problems. Small typo:
> > s/rejunk/resjunk/
>
> Thank you for the review. Attached is an updated version of the patch.
Thanks. I gave this a look, and made it some trivial adjustments.
Attached is the edited version. I think this needs some more (succint)
code comments:
. why do we want to remove these entries
. why can't we do it in the DISTINCT case
. why don't we remove the cases we don't remove, within adjust_targetlist().
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index d80c264..aa59942 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -112,6 +112,7 @@ static void get_column_info_for_window(PlannerInfo *root, WindowClause *wc,
int *ordNumCols,
AttrNumber **ordColIdx,
Oid **ordOperators);
+static List *adjust_targetlist(PlannerInfo *root, List *tlist);
/*****************************************************************************
@@ -1016,6 +1017,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
double dNumGroups = 0;
bool use_hashed_distinct = false;
bool tested_hashed_distinct = false;
+ bool tlist_is_adjustable;
/* Tweak caller-supplied tuple_fraction if have LIMIT/OFFSET */
if (parse->limitCount || parse->limitOffset)
@@ -1616,6 +1618,9 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
}
} /* end of if (setOperations) */
+ /* Check whether we can remove resjunk sort entries safely */
+ tlist_is_adjustable = is_projection_capable_plan(result_plan);
+
/*
* If there is a DISTINCT clause, add the necessary node(s).
*/
@@ -1715,6 +1720,9 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
result_plan,
current_pathkeys,
-1.0);
+
+ /* We can not remove resjunk sort entries safely */
+ tlist_is_adjustable = false;
}
result_plan = (Plan *) make_unique(result_plan,
@@ -1738,6 +1746,12 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
limit_tuples);
current_pathkeys = root->sort_pathkeys;
}
+ else
+ {
+ if (tlist_is_adjustable)
+ result_plan->targetlist =
+ adjust_targetlist(root, result_plan->targetlist);
+ }
}
/*
@@ -3388,6 +3402,128 @@ get_column_info_for_window(PlannerInfo *root, WindowClause *wc, List *tlist,
}
}
+/*
+ * adjust_targetlist
+ * Adjust a targetlist to remove useless resjunk entries
+ */
+static List *
+adjust_targetlist(PlannerInfo *root, List *tlist)
+{
+ Query *parse = root->parse;
+ Bitmapset *srefs;
+ Bitmapset *grefs;
+ ListCell *lc;
+ bool found;
+ ListCell *curr;
+ ListCell *prev;
+ ListCell *next;
+
+ /*
+ * Collect the sortgroupref numbers of ORDER BY clause into a bitmapset
+ * for convenient reference below.
+ */
+ srefs = NULL;
+ foreach(lc, parse->sortClause)
+ {
+ SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc);
+
+ srefs = bms_add_member(srefs, sortcl->tleSortGroupRef);
+ }
+
+ /*
+ * Collect the sortgroupref numbers of GROUP BY, DISTINCT ON and window
+ * PARTITION/ORDER BY clauses into a bitmapset for convenient reference
+ * below.
+ */
+ grefs = NULL;
+ if (parse->groupClause)
+ {
+ foreach(lc, parse->groupClause)
+ {
+ SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc);
+
+ grefs = bms_add_member(grefs, sortcl->tleSortGroupRef);
+ }
+ }
+ if (parse->hasDistinctOn)
+ {
+ foreach(lc, parse->distinctClause)
+ {
+ SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc);
+
+ grefs = bms_add_member(grefs, sortcl->tleSortGroupRef);
+ }
+ }
+ if (parse->hasWindowFuncs)
+ {
+ foreach(lc, parse->windowClause)
+ {
+ WindowClause *wc = (WindowClause *) lfirst(lc);
+ ListCell *lc2;
+
+ foreach(lc2, wc->partitionClause)
+ {
+ SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc2);
+
+ grefs = bms_add_member(grefs, sortcl->tleSortGroupRef);
+ }
+ foreach(lc2, wc->orderClause)
+ {
+ SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc2);
+
+ grefs = bms_add_member(grefs, sortcl->tleSortGroupRef);
+ }
+ }
+ }
+
+ /* Find the sortgroupref numbers used only for ORDER BY clause. */
+ srefs = bms_del_members(srefs, grefs);
+ if (bms_is_empty(srefs))
+ return tlist;
+
+ /* Remove resjunk sort entries, if any. */
+ curr = list_head(tlist);
+ prev = NULL;
+ found = false;
+ while (curr)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(curr);
+
+ next = lnext(curr);
+
+ if (tle->resjunk && tle->ressortgroupref != 0 &&
+ bms_is_member(tle->ressortgroupref, srefs))
+ {
+ tlist = list_delete_cell(tlist, curr, prev);
+ found = true;
+ }
+ else
+ prev = curr;
+
+ curr = next;
+ }
+
+ if (found)
+ {
+ int resno = 1;
+
+ /* Adjust resno in remaining target entries */
+ foreach(lc, tlist)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(lc);
+
+ if (tle->resno != resno)
+ {
+ tle = flatCopyTargetEntry(tle);
+ tle->resno = resno;
+ }
+ resno++;
+ }
+ }
+
+ return tlist;
+}
+
/*
* expression_planner
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
