Hi, lfirst_node() changes are missing for List node type and I was thinking about adding those. But it looks like we cannot do so as List can be a T_List, T_IntList, or T_OidList. So I am OK with that.
Apart from this, changes look good to me. Everything works fine. As we are now doing it for lfirst(), can we also do this for linitial()? I did not find any usage for lsecond(), lthird(), lfourth() and llast() though. Attached patch for replacing linitial() with linital_node() appropriately in planner.c Thanks On Fri, Jul 14, 2017 at 2:25 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > On Thu, Jul 13, 2017 at 9:15 PM, Alvaro Herrera > <alvhe...@2ndquadrant.com> wrote: > > Ashutosh Bapat wrote: > > > >> Happened to stumble across some instances of lfirst() which could use > >> lfirst_node() in planner.c. Here's patch which replaces calls to > >> lfirst() extracting node pointers by lfirst_node() in planner.c. > > > > Sounds good. > > > >> Are we carrying out such replacements in master or this will be > >> considered for v11? > > > > This is for pg11 definitely; please add to the open commitfest. > > Thanks. Added. https://commitfest.postgresql.org/14/1195/ > -- > Best Wishes, > Ashutosh Bapat > EnterpriseDB Corporation > The Postgres Database Company > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Jeevan Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation The Enterprise PostgreSQL Company
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index a1dd157..9115655 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -1556,8 +1556,8 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, /*------ translator: %s is a SQL row locking clause such as FOR UPDATE */ errmsg("%s is not allowed with UNION/INTERSECT/EXCEPT", - LCS_asString(((RowMarkClause *) - linitial(parse->rowMarks))->strength)))); + LCS_asString(linitial_node(RowMarkClause, + parse->rowMarks)->strength)))); /* * Calculate pathkeys that represent result ordering requirements @@ -1687,7 +1687,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, qp_extra.tlist = tlist; qp_extra.activeWindows = activeWindows; qp_extra.groupClause = (gset_data - ? (gset_data->rollups ? ((RollupData *) linitial(gset_data->rollups))->groupClause : NIL) + ? (gset_data->rollups ? linitial_node(RollupData, gset_data->rollups)->groupClause : NIL) : parse->groupClause); /* @@ -1757,25 +1757,25 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, split_pathtarget_at_srfs(root, final_target, sort_input_target, &final_targets, &final_targets_contain_srfs); - final_target = (PathTarget *) linitial(final_targets); + final_target = linitial_node(PathTarget, final_targets); Assert(!linitial_int(final_targets_contain_srfs)); /* likewise for sort_input_target vs. grouping_target */ split_pathtarget_at_srfs(root, sort_input_target, grouping_target, &sort_input_targets, &sort_input_targets_contain_srfs); - sort_input_target = (PathTarget *) linitial(sort_input_targets); + sort_input_target = linitial_node(PathTarget, sort_input_targets); Assert(!linitial_int(sort_input_targets_contain_srfs)); /* likewise for grouping_target vs. scanjoin_target */ split_pathtarget_at_srfs(root, grouping_target, scanjoin_target, &grouping_targets, &grouping_targets_contain_srfs); - grouping_target = (PathTarget *) linitial(grouping_targets); + grouping_target = linitial_node(PathTarget, grouping_targets); Assert(!linitial_int(grouping_targets_contain_srfs)); /* scanjoin_target will not have any SRFs precomputed for it */ split_pathtarget_at_srfs(root, scanjoin_target, NULL, &scanjoin_targets, &scanjoin_targets_contain_srfs); - scanjoin_target = (PathTarget *) linitial(scanjoin_targets); + scanjoin_target = linitial_node(PathTarget, scanjoin_targets); Assert(!linitial_int(scanjoin_targets_contain_srfs)); } else @@ -2194,7 +2194,7 @@ preprocess_grouping_sets(PlannerInfo *root) /* * Get the initial (and therefore largest) grouping set. */ - gs = linitial(current_sets); + gs = linitial_node(GroupingSetData, current_sets); /* * Order the groupClause appropriately. If the first grouping set is @@ -2344,8 +2344,8 @@ preprocess_rowmarks(PlannerInfo *root) * CTIDs invalid. This is also checked at parse time, but that's * insufficient because of rule substitution, query pullup, etc. */ - CheckSelectLocking(parse, ((RowMarkClause *) - linitial(parse->rowMarks))->strength); + CheckSelectLocking(parse, linitial_node(RowMarkClause, + parse->rowMarks)->strength); } else { @@ -3296,7 +3296,7 @@ standard_qp_callback(PlannerInfo *root, void *extra) /* We consider only the first (bottom) window in pathkeys logic */ if (activeWindows != NIL) { - WindowClause *wc = (WindowClause *) linitial(activeWindows); + WindowClause *wc = linitial_node(WindowClause, activeWindows); root->window_pathkeys = make_pathkeys_for_window(root, wc, @@ -5339,7 +5339,7 @@ select_active_windows(PlannerInfo *root, WindowFuncLists *wflists) result = NIL; while (actives != NIL) { - WindowClause *wc = (WindowClause *) linitial(actives); + WindowClause *wc = linitial_node(WindowClause, actives); ListCell *prev; ListCell *next;
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers