Replacing lfirst() with lfirst_node() appropriately in planner.c
Started by Ashutosh Bapatover 8 years ago8 messages
Hi,
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. I
have covered all the occurences of lfirst() except
1. those extract generic pointers like Path, Plan etc.
2. those extract any node as Aggref, Var and then check using IsA()
3. those extracting some pointers other than nodes.
"make check" runs without any failure.
Are we carrying out such replacements in master or this will be
considered for v11?
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Attachments:
pg_lfirst_node_planner_c.patchapplication/octet-stream; name=pg_lfirst_node_planner_c.patchDownload
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 2988c11..75b96e8 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -397,7 +397,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
forboth(lp, glob->subplans, lr, glob->subroots)
{
Plan *subplan = (Plan *) lfirst(lp);
- PlannerInfo *subroot = (PlannerInfo *) lfirst(lr);
+ PlannerInfo *subroot = lfirst_node(PlannerInfo, lr);
SS_finalize_plan(subroot, subplan);
}
@@ -416,7 +416,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
forboth(lp, glob->subplans, lr, glob->subroots)
{
Plan *subplan = (Plan *) lfirst(lp);
- PlannerInfo *subroot = (PlannerInfo *) lfirst(lr);
+ PlannerInfo *subroot = lfirst_node(PlannerInfo, lr);
lfirst(lp) = set_plan_references(subroot, subplan);
}
@@ -572,7 +572,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
hasOuterJoins = false;
foreach(l, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
if (rte->rtekind == RTE_JOIN)
{
@@ -629,7 +629,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
newWithCheckOptions = NIL;
foreach(l, parse->withCheckOptions)
{
- WithCheckOption *wco = (WithCheckOption *) lfirst(l);
+ WithCheckOption *wco = lfirst_node(WithCheckOption, l);
wco->qual = preprocess_expression(root, wco->qual,
EXPRKIND_QUAL);
@@ -649,7 +649,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
foreach(l, parse->windowClause)
{
- WindowClause *wc = (WindowClause *) lfirst(l);
+ WindowClause *wc = lfirst_node(WindowClause, l);
/* partitionClause/orderClause are sort/group expressions */
wc->startOffset = preprocess_expression(root, wc->startOffset,
@@ -691,7 +691,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
/* Also need to preprocess expressions within RTEs */
foreach(l, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
int kind;
ListCell *lcsq;
@@ -1066,7 +1066,7 @@ inheritance_planner(PlannerInfo *root)
rti = 1;
foreach(lc, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
if (rte->rtekind == RTE_SUBQUERY)
subqueryRTindexes = bms_add_member(subqueryRTindexes, rti);
@@ -1088,7 +1088,7 @@ inheritance_planner(PlannerInfo *root)
{
foreach(lc, root->append_rel_list)
{
- AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
+ AppendRelInfo *appinfo = lfirst_node(AppendRelInfo, lc);
if (bms_is_member(appinfo->parent_relid, subqueryRTindexes) ||
bms_is_member(appinfo->child_relid, subqueryRTindexes) ||
@@ -1116,7 +1116,7 @@ inheritance_planner(PlannerInfo *root)
*/
foreach(lc, root->append_rel_list)
{
- AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
+ AppendRelInfo *appinfo = lfirst_node(AppendRelInfo, lc);
PlannerInfo *subroot;
RangeTblEntry *child_rte;
RelOptInfo *sub_final_rel;
@@ -1178,7 +1178,7 @@ inheritance_planner(PlannerInfo *root)
subroot->append_rel_list = NIL;
foreach(lc2, root->append_rel_list)
{
- AppendRelInfo *appinfo2 = (AppendRelInfo *) lfirst(lc2);
+ AppendRelInfo *appinfo2 = lfirst_node(AppendRelInfo, lc2);
if (bms_is_member(appinfo2->child_relid, modifiableARIindexes))
appinfo2 = copyObject(appinfo2);
@@ -1213,7 +1213,7 @@ inheritance_planner(PlannerInfo *root)
rti = 1;
foreach(lr, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(lr);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, lr);
if (bms_is_member(rti, subqueryRTindexes))
{
@@ -1235,7 +1235,7 @@ inheritance_planner(PlannerInfo *root)
foreach(lc2, subroot->append_rel_list)
{
- AppendRelInfo *appinfo2 = (AppendRelInfo *) lfirst(lc2);
+ AppendRelInfo *appinfo2 = lfirst_node(AppendRelInfo, lc2);
if (bms_is_member(appinfo2->child_relid,
modifiableARIindexes))
@@ -1393,7 +1393,7 @@ inheritance_planner(PlannerInfo *root)
rti = 1;
foreach(lc, final_rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
root->simple_rte_array[rti++] = rte;
}
@@ -2092,7 +2092,7 @@ preprocess_grouping_sets(PlannerInfo *root)
foreach(lc, parse->groupClause)
{
- SortGroupClause *gc = lfirst(lc);
+ SortGroupClause *gc = lfirst_node(SortGroupClause, lc);
Index ref = gc->tleSortGroupRef;
if (ref > maxref)
@@ -2255,7 +2255,7 @@ remap_to_groupclause_idx(List *groupClause,
foreach(lc, groupClause)
{
- SortGroupClause *gc = lfirst(lc);
+ SortGroupClause *gc = lfirst_node(SortGroupClause, lc);
tleref_to_colnum_map[gc->tleSortGroupRef] = ref++;
}
@@ -2264,7 +2264,7 @@ remap_to_groupclause_idx(List *groupClause,
{
List *set = NIL;
ListCell *lc2;
- GroupingSetData *gs = lfirst(lc);
+ GroupingSetData *gs = lfirst_node(GroupingSetData, lc);
foreach(lc2, gs->set)
{
@@ -2359,7 +2359,7 @@ preprocess_rowmarks(PlannerInfo *root)
prowmarks = NIL;
foreach(l, parse->rowMarks)
{
- RowMarkClause *rc = (RowMarkClause *) lfirst(l);
+ RowMarkClause *rc = lfirst_node(RowMarkClause, l);
RangeTblEntry *rte = rt_fetch(rc->rti, parse->rtable);
PlanRowMark *newrc;
@@ -2399,7 +2399,7 @@ preprocess_rowmarks(PlannerInfo *root)
i = 0;
foreach(l, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
PlanRowMark *newrc;
i++;
@@ -2758,7 +2758,7 @@ remove_useless_groupby_columns(PlannerInfo *root)
(list_length(parse->rtable) + 1));
foreach(lc, parse->groupClause)
{
- SortGroupClause *sgc = (SortGroupClause *) lfirst(lc);
+ SortGroupClause *sgc = lfirst_node(SortGroupClause, lc);
TargetEntry *tle = get_sortgroupclause_tle(sgc, parse->targetList);
Var *var = (Var *) tle->expr;
@@ -2791,7 +2791,7 @@ remove_useless_groupby_columns(PlannerInfo *root)
relid = 0;
foreach(lc, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
Bitmapset *relattnos;
Bitmapset *pkattnos;
Oid constraintOid;
@@ -2849,7 +2849,7 @@ remove_useless_groupby_columns(PlannerInfo *root)
foreach(lc, parse->groupClause)
{
- SortGroupClause *sgc = (SortGroupClause *) lfirst(lc);
+ SortGroupClause *sgc = lfirst_node(SortGroupClause, lc);
TargetEntry *tle = get_sortgroupclause_tle(sgc, parse->targetList);
Var *var = (Var *) tle->expr;
@@ -2924,11 +2924,11 @@ preprocess_groupclause(PlannerInfo *root, List *force)
*/
foreach(sl, parse->sortClause)
{
- SortGroupClause *sc = (SortGroupClause *) lfirst(sl);
+ SortGroupClause *sc = lfirst_node(SortGroupClause, sl);
foreach(gl, parse->groupClause)
{
- SortGroupClause *gc = (SortGroupClause *) lfirst(gl);
+ SortGroupClause *gc = lfirst_node(SortGroupClause, gl);
if (equal(gc, sc))
{
@@ -2957,7 +2957,7 @@ preprocess_groupclause(PlannerInfo *root, List *force)
*/
foreach(gl, parse->groupClause)
{
- SortGroupClause *gc = (SortGroupClause *) lfirst(gl);
+ SortGroupClause *gc = lfirst_node(SortGroupClause, gl);
if (list_member_ptr(new_groupclause, gc))
continue; /* it matched an ORDER BY item */
@@ -3370,7 +3370,7 @@ get_number_of_groups(PlannerInfo *root,
foreach(lc, gd->rollups)
{
- RollupData *rollup = lfirst(lc);
+ RollupData *rollup = lfirst_node(RollupData, lc);
ListCell *lc;
groupExprs = get_sortgrouplist_exprs(rollup->groupClause,
@@ -3381,7 +3381,7 @@ get_number_of_groups(PlannerInfo *root,
forboth(lc, rollup->gsets, lc2, rollup->gsets_data)
{
List *gset = (List *) lfirst(lc);
- GroupingSetData *gs = lfirst(lc2);
+ GroupingSetData *gs = lfirst_node(GroupingSetData, lc2);
double numGroups = estimate_num_groups(root,
groupExprs,
path_rows,
@@ -3406,7 +3406,7 @@ get_number_of_groups(PlannerInfo *root,
forboth(lc, gd->hash_sets_idx, lc2, gd->unsortable_sets)
{
List *gset = (List *) lfirst(lc);
- GroupingSetData *gs = lfirst(lc2);
+ GroupingSetData *gs = lfirst_node(GroupingSetData, lc2);
double numGroups = estimate_num_groups(root,
groupExprs,
path_rows,
@@ -4180,7 +4180,7 @@ consider_groupingsets_paths(PlannerInfo *root,
if (pathkeys_contained_in(root->group_pathkeys, path->pathkeys))
{
- unhashed_rollup = lfirst(l_start);
+ unhashed_rollup = lfirst_node(RollupData, l_start);
exclude_groups = unhashed_rollup->numGroups;
l_start = lnext(l_start);
}
@@ -4205,7 +4205,7 @@ consider_groupingsets_paths(PlannerInfo *root,
for_each_cell(lc, l_start)
{
- RollupData *rollup = lfirst(lc);
+ RollupData *rollup = lfirst_node(RollupData, lc);
/*
* If we find an unhashable rollup that's not been skipped by the
@@ -4225,7 +4225,7 @@ consider_groupingsets_paths(PlannerInfo *root,
}
foreach(lc, sets_data)
{
- GroupingSetData *gs = lfirst(lc);
+ GroupingSetData *gs = lfirst_node(GroupingSetData, lc);
List *gset = gs->set;
RollupData *rollup;
@@ -4367,7 +4367,7 @@ consider_groupingsets_paths(PlannerInfo *root,
i = 0;
for_each_cell(lc, lnext(list_head(gd->rollups)))
{
- RollupData *rollup = lfirst(lc);
+ RollupData *rollup = lfirst_node(RollupData, lc);
if (rollup->hashable)
{
@@ -4401,7 +4401,7 @@ consider_groupingsets_paths(PlannerInfo *root,
i = 0;
for_each_cell(lc, lnext(list_head(gd->rollups)))
{
- RollupData *rollup = lfirst(lc);
+ RollupData *rollup = lfirst_node(RollupData, lc);
if (rollup->hashable)
{
@@ -4423,7 +4423,7 @@ consider_groupingsets_paths(PlannerInfo *root,
foreach(lc, hash_sets)
{
- GroupingSetData *gs = lfirst(lc);
+ GroupingSetData *gs = lfirst_node(GroupingSetData, lc);
RollupData *rollup = makeNode(RollupData);
Assert(gs->set != NIL);
@@ -4602,7 +4602,7 @@ create_one_window_path(PlannerInfo *root,
foreach(l, activeWindows)
{
- WindowClause *wc = (WindowClause *) lfirst(l);
+ WindowClause *wc = lfirst_node(WindowClause, l);
List *window_pathkeys;
window_pathkeys = make_pathkeys_for_window(root,
@@ -5189,7 +5189,7 @@ make_partial_grouping_target(PlannerInfo *root, PathTarget *grouping_target)
*/
foreach(lc, partial_target->exprs)
{
- Aggref *aggref = (Aggref *) lfirst(lc);
+ Aggref *aggref = lfirst(lc);
if (IsA(aggref, Aggref))
{
@@ -5266,7 +5266,7 @@ postprocess_setop_tlist(List *new_tlist, List *orig_tlist)
foreach(l, new_tlist)
{
- TargetEntry *new_tle = (TargetEntry *) lfirst(l);
+ TargetEntry *new_tle = lfirst_node(TargetEntry, l);
TargetEntry *orig_tle;
/* ignore resjunk columns in setop result */
@@ -5274,7 +5274,7 @@ postprocess_setop_tlist(List *new_tlist, List *orig_tlist)
continue;
Assert(orig_tlist_item != NULL);
- orig_tle = (TargetEntry *) lfirst(orig_tlist_item);
+ orig_tle = lfirst_node(TargetEntry, orig_tlist_item);
orig_tlist_item = lnext(orig_tlist_item);
if (orig_tle->resjunk) /* should not happen */
elog(ERROR, "resjunk output columns are not implemented");
@@ -5302,7 +5302,7 @@ select_active_windows(PlannerInfo *root, WindowFuncLists *wflists)
actives = NIL;
foreach(lc, root->parse->windowClause)
{
- WindowClause *wc = (WindowClause *) lfirst(lc);
+ WindowClause *wc = lfirst_node(WindowClause, lc);
/* It's only active if wflists shows some related WindowFuncs */
Assert(wc->winref <= wflists->maxWinRef);
@@ -5337,7 +5337,7 @@ select_active_windows(PlannerInfo *root, WindowFuncLists *wflists)
prev = NULL;
for (lc = list_head(actives); lc; lc = next)
{
- WindowClause *wc2 = (WindowClause *) lfirst(lc);
+ WindowClause *wc2 = lfirst_node(WindowClause, lc);
next = lnext(lc);
/* framing options are NOT to be compared here! */
@@ -5410,18 +5410,18 @@ make_window_input_target(PlannerInfo *root,
sgrefs = NULL;
foreach(lc, activeWindows)
{
- WindowClause *wc = (WindowClause *) lfirst(lc);
+ WindowClause *wc = lfirst_node(WindowClause, lc);
ListCell *lc2;
foreach(lc2, wc->partitionClause)
{
- SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc2);
+ SortGroupClause *sortcl = lfirst_node(SortGroupClause, lc2);
sgrefs = bms_add_member(sgrefs, sortcl->tleSortGroupRef);
}
foreach(lc2, wc->orderClause)
{
- SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc2);
+ SortGroupClause *sortcl = lfirst_node(SortGroupClause, lc2);
sgrefs = bms_add_member(sgrefs, sortcl->tleSortGroupRef);
}
@@ -5430,7 +5430,7 @@ make_window_input_target(PlannerInfo *root,
/* Add in sortgroupref numbers of GROUP BY clauses, too */
foreach(lc, parse->groupClause)
{
- SortGroupClause *grpcl = (SortGroupClause *) lfirst(lc);
+ SortGroupClause *grpcl = lfirst_node(SortGroupClause, lc);
sgrefs = bms_add_member(sgrefs, grpcl->tleSortGroupRef);
}
@@ -5850,7 +5850,7 @@ adjust_paths_for_srfs(PlannerInfo *root, RelOptInfo *rel,
Assert(subpath->param_info == NULL);
forboth(lc1, targets, lc2, targets_contain_srfs)
{
- PathTarget *thistarget = (PathTarget *) lfirst(lc1);
+ PathTarget *thistarget = lfirst_node(PathTarget, lc1);
bool contains_srfs = (bool) lfirst_int(lc2);
/* If this level doesn't contain SRFs, do regular projection */
@@ -5883,7 +5883,7 @@ adjust_paths_for_srfs(PlannerInfo *root, RelOptInfo *rel,
Assert(subpath->param_info == NULL);
forboth(lc1, targets, lc2, targets_contain_srfs)
{
- PathTarget *thistarget = (PathTarget *) lfirst(lc1);
+ PathTarget *thistarget = lfirst_node(PathTarget, lc1);
bool contains_srfs = (bool) lfirst_int(lc2);
/* If this level doesn't contain SRFs, do regular projection */
@@ -6009,7 +6009,7 @@ plan_cluster_use_sort(Oid tableOid, Oid indexOid)
indexInfo = NULL;
foreach(lc, rel->indexlist)
{
- indexInfo = (IndexOptInfo *) lfirst(lc);
+ indexInfo = lfirst_node(IndexOptInfo, lc);
if (indexInfo->indexoid == indexOid)
break;
}
@@ -6072,7 +6072,7 @@ get_partitioned_child_rels(PlannerInfo *root, Index rti)
foreach(l, root->pcinfo_list)
{
- PartitionedChildRelInfo *pc = lfirst(l);
+ PartitionedChildRelInfo *pc = lfirst_node(PartitionedChildRelInfo, l);
if (pc->parent_relid == rti)
{
Re: Replacing lfirst() with lfirst_node() appropriately in planner.c
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.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Replacing lfirst() with lfirst_node() appropriately in planner.c
On Thu, Jul 13, 2017 at 9:15 PM, Alvaro Herrera
<alvherre@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
Re: Replacing lfirst() with lfirst_node() appropriately in planner.c
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.bapat@enterprisedb.com> wrote:
On Thu, Jul 13, 2017 at 9:15 PM, Alvaro Herrera
<alvherre@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
Attachments:
pg_linitial_node_planner_c.patchtext/x-patch; charset=US-ASCII; name=pg_linitial_node_planner_c.patchDownload
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;
Re: Replacing lfirst() with lfirst_node() appropriately in planner.c
On Tue, Sep 5, 2017 at 4:02 PM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:
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.
Thanks for investigating the case of T_List.
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
Ok. The patch looks good to me. It compiles and make check passes.
Here are both the patches rebased on the latest sources.
I am marking this commitfest entry as "ready for committer".
Attachments:
0001-Use-lfirst_node-instead-of-lfirst-wherever-suitable-_v2.patchtext/x-patch; charset=US-ASCII; name=0001-Use-lfirst_node-instead-of-lfirst-wherever-suitable-_v2.patchDownload
From 25ae53d7f72a54a03ec90206c7e5579a562a121c Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Tue, 5 Sep 2017 16:50:58 +0530
Subject: [PATCH 1/2] Use lfirst_node() instead of lfirst() wherever suitable
in planner.c
Ashutosh Bapat, reviewed by Jeevan Chalke
---
src/backend/optimizer/plan/planner.c | 94 +++++++++++++++++-----------------
1 file changed, 47 insertions(+), 47 deletions(-)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 9662302..a1dd157 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -411,7 +411,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
forboth(lp, glob->subplans, lr, glob->subroots)
{
Plan *subplan = (Plan *) lfirst(lp);
- PlannerInfo *subroot = (PlannerInfo *) lfirst(lr);
+ PlannerInfo *subroot = lfirst_node(PlannerInfo, lr);
SS_finalize_plan(subroot, subplan);
}
@@ -430,7 +430,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
forboth(lp, glob->subplans, lr, glob->subroots)
{
Plan *subplan = (Plan *) lfirst(lp);
- PlannerInfo *subroot = (PlannerInfo *) lfirst(lr);
+ PlannerInfo *subroot = lfirst_node(PlannerInfo, lr);
lfirst(lp) = set_plan_references(subroot, subplan);
}
@@ -586,7 +586,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
hasOuterJoins = false;
foreach(l, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
if (rte->rtekind == RTE_JOIN)
{
@@ -643,7 +643,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
newWithCheckOptions = NIL;
foreach(l, parse->withCheckOptions)
{
- WithCheckOption *wco = (WithCheckOption *) lfirst(l);
+ WithCheckOption *wco = lfirst_node(WithCheckOption, l);
wco->qual = preprocess_expression(root, wco->qual,
EXPRKIND_QUAL);
@@ -663,7 +663,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
foreach(l, parse->windowClause)
{
- WindowClause *wc = (WindowClause *) lfirst(l);
+ WindowClause *wc = lfirst_node(WindowClause, l);
/* partitionClause/orderClause are sort/group expressions */
wc->startOffset = preprocess_expression(root, wc->startOffset,
@@ -705,7 +705,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
/* Also need to preprocess expressions within RTEs */
foreach(l, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
int kind;
ListCell *lcsq;
@@ -1080,7 +1080,7 @@ inheritance_planner(PlannerInfo *root)
rti = 1;
foreach(lc, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
if (rte->rtekind == RTE_SUBQUERY)
subqueryRTindexes = bms_add_member(subqueryRTindexes, rti);
@@ -1102,7 +1102,7 @@ inheritance_planner(PlannerInfo *root)
{
foreach(lc, root->append_rel_list)
{
- AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
+ AppendRelInfo *appinfo = lfirst_node(AppendRelInfo, lc);
if (bms_is_member(appinfo->parent_relid, subqueryRTindexes) ||
bms_is_member(appinfo->child_relid, subqueryRTindexes) ||
@@ -1130,7 +1130,7 @@ inheritance_planner(PlannerInfo *root)
*/
foreach(lc, root->append_rel_list)
{
- AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
+ AppendRelInfo *appinfo = lfirst_node(AppendRelInfo, lc);
PlannerInfo *subroot;
RangeTblEntry *child_rte;
RelOptInfo *sub_final_rel;
@@ -1192,7 +1192,7 @@ inheritance_planner(PlannerInfo *root)
subroot->append_rel_list = NIL;
foreach(lc2, root->append_rel_list)
{
- AppendRelInfo *appinfo2 = (AppendRelInfo *) lfirst(lc2);
+ AppendRelInfo *appinfo2 = lfirst_node(AppendRelInfo, lc2);
if (bms_is_member(appinfo2->child_relid, modifiableARIindexes))
appinfo2 = copyObject(appinfo2);
@@ -1227,7 +1227,7 @@ inheritance_planner(PlannerInfo *root)
rti = 1;
foreach(lr, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(lr);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, lr);
if (bms_is_member(rti, subqueryRTindexes))
{
@@ -1249,7 +1249,7 @@ inheritance_planner(PlannerInfo *root)
foreach(lc2, subroot->append_rel_list)
{
- AppendRelInfo *appinfo2 = (AppendRelInfo *) lfirst(lc2);
+ AppendRelInfo *appinfo2 = lfirst_node(AppendRelInfo, lc2);
if (bms_is_member(appinfo2->child_relid,
modifiableARIindexes))
@@ -1407,7 +1407,7 @@ inheritance_planner(PlannerInfo *root)
rti = 1;
foreach(lc, final_rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
root->simple_rte_array[rti++] = rte;
}
@@ -2106,7 +2106,7 @@ preprocess_grouping_sets(PlannerInfo *root)
foreach(lc, parse->groupClause)
{
- SortGroupClause *gc = lfirst(lc);
+ SortGroupClause *gc = lfirst_node(SortGroupClause, lc);
Index ref = gc->tleSortGroupRef;
if (ref > maxref)
@@ -2269,7 +2269,7 @@ remap_to_groupclause_idx(List *groupClause,
foreach(lc, groupClause)
{
- SortGroupClause *gc = lfirst(lc);
+ SortGroupClause *gc = lfirst_node(SortGroupClause, lc);
tleref_to_colnum_map[gc->tleSortGroupRef] = ref++;
}
@@ -2278,7 +2278,7 @@ remap_to_groupclause_idx(List *groupClause,
{
List *set = NIL;
ListCell *lc2;
- GroupingSetData *gs = lfirst(lc);
+ GroupingSetData *gs = lfirst_node(GroupingSetData, lc);
foreach(lc2, gs->set)
{
@@ -2373,7 +2373,7 @@ preprocess_rowmarks(PlannerInfo *root)
prowmarks = NIL;
foreach(l, parse->rowMarks)
{
- RowMarkClause *rc = (RowMarkClause *) lfirst(l);
+ RowMarkClause *rc = lfirst_node(RowMarkClause, l);
RangeTblEntry *rte = rt_fetch(rc->rti, parse->rtable);
PlanRowMark *newrc;
@@ -2413,7 +2413,7 @@ preprocess_rowmarks(PlannerInfo *root)
i = 0;
foreach(l, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
PlanRowMark *newrc;
i++;
@@ -2772,7 +2772,7 @@ remove_useless_groupby_columns(PlannerInfo *root)
(list_length(parse->rtable) + 1));
foreach(lc, parse->groupClause)
{
- SortGroupClause *sgc = (SortGroupClause *) lfirst(lc);
+ SortGroupClause *sgc = lfirst_node(SortGroupClause, lc);
TargetEntry *tle = get_sortgroupclause_tle(sgc, parse->targetList);
Var *var = (Var *) tle->expr;
@@ -2805,7 +2805,7 @@ remove_useless_groupby_columns(PlannerInfo *root)
relid = 0;
foreach(lc, parse->rtable)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
Bitmapset *relattnos;
Bitmapset *pkattnos;
Oid constraintOid;
@@ -2863,7 +2863,7 @@ remove_useless_groupby_columns(PlannerInfo *root)
foreach(lc, parse->groupClause)
{
- SortGroupClause *sgc = (SortGroupClause *) lfirst(lc);
+ SortGroupClause *sgc = lfirst_node(SortGroupClause, lc);
TargetEntry *tle = get_sortgroupclause_tle(sgc, parse->targetList);
Var *var = (Var *) tle->expr;
@@ -2938,11 +2938,11 @@ preprocess_groupclause(PlannerInfo *root, List *force)
*/
foreach(sl, parse->sortClause)
{
- SortGroupClause *sc = (SortGroupClause *) lfirst(sl);
+ SortGroupClause *sc = lfirst_node(SortGroupClause, sl);
foreach(gl, parse->groupClause)
{
- SortGroupClause *gc = (SortGroupClause *) lfirst(gl);
+ SortGroupClause *gc = lfirst_node(SortGroupClause, gl);
if (equal(gc, sc))
{
@@ -2971,7 +2971,7 @@ preprocess_groupclause(PlannerInfo *root, List *force)
*/
foreach(gl, parse->groupClause)
{
- SortGroupClause *gc = (SortGroupClause *) lfirst(gl);
+ SortGroupClause *gc = lfirst_node(SortGroupClause, gl);
if (list_member_ptr(new_groupclause, gc))
continue; /* it matched an ORDER BY item */
@@ -3384,7 +3384,7 @@ get_number_of_groups(PlannerInfo *root,
foreach(lc, gd->rollups)
{
- RollupData *rollup = lfirst(lc);
+ RollupData *rollup = lfirst_node(RollupData, lc);
ListCell *lc;
groupExprs = get_sortgrouplist_exprs(rollup->groupClause,
@@ -3395,7 +3395,7 @@ get_number_of_groups(PlannerInfo *root,
forboth(lc, rollup->gsets, lc2, rollup->gsets_data)
{
List *gset = (List *) lfirst(lc);
- GroupingSetData *gs = lfirst(lc2);
+ GroupingSetData *gs = lfirst_node(GroupingSetData, lc2);
double numGroups = estimate_num_groups(root,
groupExprs,
path_rows,
@@ -3420,7 +3420,7 @@ get_number_of_groups(PlannerInfo *root,
forboth(lc, gd->hash_sets_idx, lc2, gd->unsortable_sets)
{
List *gset = (List *) lfirst(lc);
- GroupingSetData *gs = lfirst(lc2);
+ GroupingSetData *gs = lfirst_node(GroupingSetData, lc2);
double numGroups = estimate_num_groups(root,
groupExprs,
path_rows,
@@ -4194,7 +4194,7 @@ consider_groupingsets_paths(PlannerInfo *root,
if (pathkeys_contained_in(root->group_pathkeys, path->pathkeys))
{
- unhashed_rollup = lfirst(l_start);
+ unhashed_rollup = lfirst_node(RollupData, l_start);
exclude_groups = unhashed_rollup->numGroups;
l_start = lnext(l_start);
}
@@ -4219,7 +4219,7 @@ consider_groupingsets_paths(PlannerInfo *root,
for_each_cell(lc, l_start)
{
- RollupData *rollup = lfirst(lc);
+ RollupData *rollup = lfirst_node(RollupData, lc);
/*
* If we find an unhashable rollup that's not been skipped by the
@@ -4239,7 +4239,7 @@ consider_groupingsets_paths(PlannerInfo *root,
}
foreach(lc, sets_data)
{
- GroupingSetData *gs = lfirst(lc);
+ GroupingSetData *gs = lfirst_node(GroupingSetData, lc);
List *gset = gs->set;
RollupData *rollup;
@@ -4381,7 +4381,7 @@ consider_groupingsets_paths(PlannerInfo *root,
i = 0;
for_each_cell(lc, lnext(list_head(gd->rollups)))
{
- RollupData *rollup = lfirst(lc);
+ RollupData *rollup = lfirst_node(RollupData, lc);
if (rollup->hashable)
{
@@ -4415,7 +4415,7 @@ consider_groupingsets_paths(PlannerInfo *root,
i = 0;
for_each_cell(lc, lnext(list_head(gd->rollups)))
{
- RollupData *rollup = lfirst(lc);
+ RollupData *rollup = lfirst_node(RollupData, lc);
if (rollup->hashable)
{
@@ -4437,7 +4437,7 @@ consider_groupingsets_paths(PlannerInfo *root,
foreach(lc, hash_sets)
{
- GroupingSetData *gs = lfirst(lc);
+ GroupingSetData *gs = lfirst_node(GroupingSetData, lc);
RollupData *rollup = makeNode(RollupData);
Assert(gs->set != NIL);
@@ -4616,7 +4616,7 @@ create_one_window_path(PlannerInfo *root,
foreach(l, activeWindows)
{
- WindowClause *wc = (WindowClause *) lfirst(l);
+ WindowClause *wc = lfirst_node(WindowClause, l);
List *window_pathkeys;
window_pathkeys = make_pathkeys_for_window(root,
@@ -5203,7 +5203,7 @@ make_partial_grouping_target(PlannerInfo *root, PathTarget *grouping_target)
*/
foreach(lc, partial_target->exprs)
{
- Aggref *aggref = (Aggref *) lfirst(lc);
+ Aggref *aggref = lfirst(lc);
if (IsA(aggref, Aggref))
{
@@ -5280,7 +5280,7 @@ postprocess_setop_tlist(List *new_tlist, List *orig_tlist)
foreach(l, new_tlist)
{
- TargetEntry *new_tle = (TargetEntry *) lfirst(l);
+ TargetEntry *new_tle = lfirst_node(TargetEntry, l);
TargetEntry *orig_tle;
/* ignore resjunk columns in setop result */
@@ -5288,7 +5288,7 @@ postprocess_setop_tlist(List *new_tlist, List *orig_tlist)
continue;
Assert(orig_tlist_item != NULL);
- orig_tle = (TargetEntry *) lfirst(orig_tlist_item);
+ orig_tle = lfirst_node(TargetEntry, orig_tlist_item);
orig_tlist_item = lnext(orig_tlist_item);
if (orig_tle->resjunk) /* should not happen */
elog(ERROR, "resjunk output columns are not implemented");
@@ -5316,7 +5316,7 @@ select_active_windows(PlannerInfo *root, WindowFuncLists *wflists)
actives = NIL;
foreach(lc, root->parse->windowClause)
{
- WindowClause *wc = (WindowClause *) lfirst(lc);
+ WindowClause *wc = lfirst_node(WindowClause, lc);
/* It's only active if wflists shows some related WindowFuncs */
Assert(wc->winref <= wflists->maxWinRef);
@@ -5351,7 +5351,7 @@ select_active_windows(PlannerInfo *root, WindowFuncLists *wflists)
prev = NULL;
for (lc = list_head(actives); lc; lc = next)
{
- WindowClause *wc2 = (WindowClause *) lfirst(lc);
+ WindowClause *wc2 = lfirst_node(WindowClause, lc);
next = lnext(lc);
/* framing options are NOT to be compared here! */
@@ -5424,18 +5424,18 @@ make_window_input_target(PlannerInfo *root,
sgrefs = NULL;
foreach(lc, activeWindows)
{
- WindowClause *wc = (WindowClause *) lfirst(lc);
+ WindowClause *wc = lfirst_node(WindowClause, lc);
ListCell *lc2;
foreach(lc2, wc->partitionClause)
{
- SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc2);
+ SortGroupClause *sortcl = lfirst_node(SortGroupClause, lc2);
sgrefs = bms_add_member(sgrefs, sortcl->tleSortGroupRef);
}
foreach(lc2, wc->orderClause)
{
- SortGroupClause *sortcl = (SortGroupClause *) lfirst(lc2);
+ SortGroupClause *sortcl = lfirst_node(SortGroupClause, lc2);
sgrefs = bms_add_member(sgrefs, sortcl->tleSortGroupRef);
}
@@ -5444,7 +5444,7 @@ make_window_input_target(PlannerInfo *root,
/* Add in sortgroupref numbers of GROUP BY clauses, too */
foreach(lc, parse->groupClause)
{
- SortGroupClause *grpcl = (SortGroupClause *) lfirst(lc);
+ SortGroupClause *grpcl = lfirst_node(SortGroupClause, lc);
sgrefs = bms_add_member(sgrefs, grpcl->tleSortGroupRef);
}
@@ -5864,7 +5864,7 @@ adjust_paths_for_srfs(PlannerInfo *root, RelOptInfo *rel,
Assert(subpath->param_info == NULL);
forboth(lc1, targets, lc2, targets_contain_srfs)
{
- PathTarget *thistarget = (PathTarget *) lfirst(lc1);
+ PathTarget *thistarget = lfirst_node(PathTarget, lc1);
bool contains_srfs = (bool) lfirst_int(lc2);
/* If this level doesn't contain SRFs, do regular projection */
@@ -5897,7 +5897,7 @@ adjust_paths_for_srfs(PlannerInfo *root, RelOptInfo *rel,
Assert(subpath->param_info == NULL);
forboth(lc1, targets, lc2, targets_contain_srfs)
{
- PathTarget *thistarget = (PathTarget *) lfirst(lc1);
+ PathTarget *thistarget = lfirst_node(PathTarget, lc1);
bool contains_srfs = (bool) lfirst_int(lc2);
/* If this level doesn't contain SRFs, do regular projection */
@@ -6023,7 +6023,7 @@ plan_cluster_use_sort(Oid tableOid, Oid indexOid)
indexInfo = NULL;
foreach(lc, rel->indexlist)
{
- indexInfo = (IndexOptInfo *) lfirst(lc);
+ indexInfo = lfirst_node(IndexOptInfo, lc);
if (indexInfo->indexoid == indexOid)
break;
}
@@ -6086,7 +6086,7 @@ get_partitioned_child_rels(PlannerInfo *root, Index rti)
foreach(l, root->pcinfo_list)
{
- PartitionedChildRelInfo *pc = lfirst(l);
+ PartitionedChildRelInfo *pc = lfirst_node(PartitionedChildRelInfo, l);
if (pc->parent_relid == rti)
{
--
1.7.9.5
0002-Use-linitial_node-instead-of-linitial-wherever-suita_v2.patchtext/x-patch; charset=US-ASCII; name=0002-Use-linitial_node-instead-of-linitial-wherever-suita_v2.patchDownload
From ddfe968ec316793cbbf8787e9b73d86921077606 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Tue, 5 Sep 2017 17:20:04 +0530
Subject: [PATCH 2/2] Use linitial_node() instead of linitial() wherever
suitable in planner.c
Jeevan Chalke, reviewed by Ashutosh Bapat
---
src/backend/optimizer/plan/planner.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
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;
--
1.7.9.5
Re: Replacing lfirst() with lfirst_node() appropriately in planner.c
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
On Tue, Sep 5, 2017 at 4:02 PM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:Attached patch for replacing linitial() with linital_node() appropriately in
planner.c
Ok. The patch looks good to me. It compiles and make check passes.
Here are both the patches rebased on the latest sources.
LGTM, pushed. I also changed a couple of places that left off any cast
of lfirst, eg:
- List *gset = lfirst(lc);
+ List *gset = (List *) lfirst(lc);While this isn't really harmful, it's not per prevailing style.
BTW, I think we *could* use "lfirst_node(List, ...)" in cases where
we know the list is supposed to be a list of objects rather than ints
or Oids. I didn't do anything about that observation, though.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Replacing lfirst() with lfirst_node() appropriately in planner.c
On Wed, Sep 6, 2017 at 1:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
On Tue, Sep 5, 2017 at 4:02 PM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:Attached patch for replacing linitial() with linital_node() appropriately in
planner.cOk. The patch looks good to me. It compiles and make check passes.
Here are both the patches rebased on the latest sources.LGTM, pushed. I also changed a couple of places that left off any cast
of lfirst, eg:- List *gset = lfirst(lc); + List *gset = (List *) lfirst(lc);While this isn't really harmful, it's not per prevailing style.
+1. Thanks.
BTW, I think we *could* use "lfirst_node(List, ...)" in cases where
we know the list is supposed to be a list of objects rather than ints
or Oids. I didn't do anything about that observation, though.
IMO, it won't be apparent as to why some code uses lfirst_node(List,
...) and some code uses (List *) lfirst().
--
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
Re: Replacing lfirst() with lfirst_node() appropriately in planner.c
Ashutosh Bapat wrote:
On Wed, Sep 6, 2017 at 1:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
BTW, I think we *could* use "lfirst_node(List, ...)" in cases where
we know the list is supposed to be a list of objects rather than ints
or Oids. I didn't do anything about that observation, though.IMO, it won't be apparent as to why some code uses lfirst_node(List,
...) and some code uses (List *) lfirst().
Yeah -- based on that argument, I too think we should leave those alone.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers