Remove field UpdateContext->updated in nodeModifyTable.c

deanrasheed · deanrasheed · commit 362de947cd7e · 2024-02-29T11:49:30.000Z
This field has been redundant ever since it was added by commit 25e777c, which split up ExecUpdate() and ExecDelete() into reusable pieces. The only place that reads it is ExecMergeMatched(), if the result from ExecUpdateAct() is TM_Ok. However, all paths through ExecUpdateAct() that return TM_Ok also set this field to true, so the return status by itself is sufficient to tell if the update happened. Removing this field is a modest simplification, and it brings the UPDATE path in ExecMergeMatched() more into line with ExecUpdate(), ensuring that ExecUpdateEpilogue() is always called if ExecUpdateAct() returns TM_Ok, reducing the chance of bugs. Dean Rasheed, reviewed by Alvaro Herrera. Discussion: https://postgr.es/m/CAEZATCWGGmigGBzLHkJm5Ccv2mMxXmwi3%2Buq0yhwDHm-tsvSLg%40mail.gmail.com
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
@@ -109,7 +109,6 @@ typedef struct ModifyTableContext
  */
 typedef struct UpdateContext
 {
-	bool		updated;		/* did UPDATE actually occur? */
 	bool		crossPartUpdate;	/* was it a cross-partition update? */
 	TU_UpdateIndexes updateIndexes; /* Which index updates are required? */
 
@@ -2042,7 +2041,6 @@ ExecUpdateAct(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
 									 &insert_destrel))
 		{
 			/* success! */
-			updateCxt->updated = true;
 			updateCxt->crossPartUpdate = true;
 
 			/*
@@ -2111,8 +2109,6 @@ ExecUpdateAct(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
 								true /* wait for commit */ ,
 								&context->tmfd, &updateCxt->lockmode,
 								&updateCxt->updateIndexes);
-	if (result == TM_Ok)
-		updateCxt->updated = true;
 
 	return result;
 }
@@ -2919,7 +2915,7 @@ ExecMergeMatched(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
 					return true;
 				}
 
-				if (result == TM_Ok && updateCxt.updated)
+				if (result == TM_Ok)
 				{
 					ExecUpdateEpilogue(context, &updateCxt, resultRelInfo,
 									   tupleid, NULL, newslot);

Original file line number	Diff line number	Diff line change
`@@ -109,7 +109,6 @@ typedef struct ModifyTableContext`
`109`	`109`	`*/`
`110`	`110`	`typedef struct UpdateContext`
`111`	`111`	`{`
`112`		`- bool updated; /* did UPDATE actually occur? */`
`113`	`112`	`bool crossPartUpdate; /* was it a cross-partition update? */`
`114`	`113`	`TU_UpdateIndexes updateIndexes; /* Which index updates are required? */`
`115`	`114`
`@@ -2042,7 +2041,6 @@ ExecUpdateAct(ModifyTableContext context, ResultRelInfo resultRelInfo,`
`2042`	`2041`	`&insert_destrel))`
`2043`	`2042`	`{`
`2044`	`2043`	`/* success! */`
`2045`		`- updateCxt->updated = true;`
`2046`	`2044`	`updateCxt->crossPartUpdate = true;`
`2047`	`2045`
`2048`	`2046`	`/*`
`@@ -2111,8 +2109,6 @@ ExecUpdateAct(ModifyTableContext context, ResultRelInfo resultRelInfo,`
`2111`	`2109`	`true /* wait for commit */ ,`
`2112`	`2110`	`&context->tmfd, &updateCxt->lockmode,`
`2113`	`2111`	`&updateCxt->updateIndexes);`
`2114`		`- if (result == TM_Ok)`
`2115`		`- updateCxt->updated = true;`
`2116`	`2112`
`2117`	`2113`	`return result;`
`2118`	`2114`	`}`
`@@ -2919,7 +2915,7 @@ ExecMergeMatched(ModifyTableContext context, ResultRelInfo resultRelInfo,`
`2919`	`2915`	`return true;`
`2920`	`2916`	`}`
`2921`	`2917`
`2922`		`- if (result == TM_Ok && updateCxt.updated)`
	`2918`	`+ if (result == TM_Ok)`
`2923`	`2919`	`{`
`2924`	`2920`	`ExecUpdateEpilogue(context, &updateCxt, resultRelInfo,`
`2925`	`2921`	`tupleid, NULL, newslot);`