Optimize a few list_delete_ptr calls

There is a handful of places where we called list_delete_ptr() to remove
some element from a List.  In many of these places we know, or with very
little additional effort know the index of the ListCell that we need to
remove.

Here we change all of those places to instead either use one of;
list_delete_nth_cell(), foreach_delete_current() or list_delete_last().
Each of these saves from having to iterate over the list to search for the
element to remove by its pointer value.

There are some small performance gains to be had by doing this, but in the
general case, none of these lists are likely to be very large, so the
lookup was probably never that expensive anyway.  However, some of the
calls are in fairly hot code paths, e.g process_equivalence().  So any
small gains there are useful.

Author: Zhijie Hou and David Rowley
Discussion: https://postgr.es/m/b3517353ec7c4f87aa560678fbb1034b@G08CNEXMBPEKD05.g08.fujitsu.local

src/backend/optimizer/path/equivclass.c

@@ -137,6 +137,7 @@ process_equivalence(PlannerInfo *root,
 	EquivalenceMember *em1,
 			   *em2;
 	ListCell   *lc1;
+	int			ec2_idx;
 
 	/* Should not already be marked as having generated an eclass */
 	Assert(restrictinfo->left_ec == NULL);
@@ -258,6 +259,7 @@ process_equivalence(PlannerInfo *root,
 	 */
 	ec1 = ec2 = NULL;
 	em1 = em2 = NULL;
+	ec2_idx = -1;
 	foreach(lc1, root->eq_classes)
 	{
 		EquivalenceClass *cur_ec = (EquivalenceClass *) lfirst(lc1);
@@ -311,6 +313,7 @@ process_equivalence(PlannerInfo *root,
 				equal(item2, cur_em->em_expr))
 			{
 				ec2 = cur_ec;
+				ec2_idx = foreach_current_index(lc1);
 				em2 = cur_em;
 				if (ec1)
 					break;
@@ -371,7 +374,7 @@ process_equivalence(PlannerInfo *root,
 		ec1->ec_max_security = Max(ec1->ec_max_security,
 								   ec2->ec_max_security);
 		ec2->ec_merged = ec1;
-		root->eq_classes = list_delete_ptr(root->eq_classes, ec2);
+		root->eq_classes = list_delete_nth_cell(root->eq_classes, ec2_idx);
 		/* just to avoid debugging confusion w/ dangling pointers: */
 		ec2->ec_members = NIL;
 		ec2->ec_sources = NIL;
@@ -1964,6 +1967,7 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
 		bool		matchleft;
 		bool		matchright;
 		ListCell   *lc2;
+		int			coal_idx = -1;
 
 		/* Ignore EC unless it contains pseudoconstants */
 		if (!cur_ec->ec_has_const)
@@ -2008,6 +2012,7 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
 
 				if (equal(leftvar, cfirst) && equal(rightvar, csecond))
 				{
+					coal_idx = foreach_current_index(lc2);
 					match = true;
 					break;
 				}
@@ -2072,7 +2077,7 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
 		 */
 		if (matchleft && matchright)
 		{
-			cur_ec->ec_members = list_delete_ptr(cur_ec->ec_members, coal_em);
+			cur_ec->ec_members = list_delete_nth_cell(cur_ec->ec_members, coal_idx);
 			return true;
 		}
 

src/backend/optimizer/path/joinpath.c

@@ -1005,8 +1005,8 @@ sort_inner_and_outer(PlannerInfo *root,
 		/* Make a pathkey list with this guy first */
 		if (l != list_head(all_pathkeys))
 			outerkeys = lcons(front_pathkey,
-							  list_delete_ptr(list_copy(all_pathkeys),
-											  front_pathkey));
+							  list_delete_nth_cell(list_copy(all_pathkeys),
+												   foreach_current_index(l)));
 		else
 			outerkeys = all_pathkeys;	/* no work at first one... */
 

src/backend/parser/parse_expr.c

@@ -1698,11 +1698,12 @@ transformMultiAssignRef(ParseState *pstate, MultiAssignRef *maref)
 		/*
 		 * If we're at the last column, delete the RowExpr from
 		 * p_multiassign_exprs; we don't need it anymore, and don't want it in
-		 * the finished UPDATE tlist.
+		 * the finished UPDATE tlist.  We assume this is still the last entry
+		 * in p_multiassign_exprs.
 		 */
 		if (maref->colno == maref->ncolumns)
 			pstate->p_multiassign_exprs =
-				list_delete_ptr(pstate->p_multiassign_exprs, tle);
+				list_delete_last(pstate->p_multiassign_exprs);
 
 		return result;
 	}

src/backend/parser/parse_utilcmd.c

@@ -360,6 +360,7 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
 	CreateSeqStmt *seqstmt;
 	AlterSeqStmt *altseqstmt;
 	List	   *attnamelist;
+	int			nameEl_idx = -1;
 
 	/*
 	 * Determine namespace and name to use for the sequence.
@@ -386,6 +387,7 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("conflicting or redundant options")));
 			nameEl = defel;
+			nameEl_idx = foreach_current_index(option);
 		}
 	}
 
@@ -405,7 +407,7 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
 		}
 		sname = rv->relname;
 		/* Remove the SEQUENCE NAME item from seqoptions */
-		seqoptions = list_delete_ptr(seqoptions, nameEl);
+		seqoptions = list_delete_nth_cell(seqoptions, nameEl_idx);
 	}
 	else
 	{

src/backend/rewrite/rewriteHandler.c

@@ -650,11 +650,7 @@ adjustJoinTreeList(Query *parsetree, bool removert, int rt_index)
 			if (IsA(rtr, RangeTblRef) &&
 				rtr->rtindex == rt_index)
 			{
-				newjointree = list_delete_ptr(newjointree, rtr);
-
-				/*
-				 * foreach is safe because we exit loop after list_delete...
-				 */
+				newjointree = foreach_delete_current(newjointree, l);
 				break;
 			}
 		}