Make ExecGetInsertedCols() and friends more robust and improve comments.

hlinnaka · hlinnaka · commit 6e437ce5a6b6 · 2021-02-15T09:29:49.000+02:00
If ExecGetInsertedCols(), ExecGetUpdatedCols() or ExecGetExtraUpdatedCols() were called with a ResultRelInfo that's not in the range table and isn't a partition routing target, the functions would dereference a NULL pointer, relinfo->ri_RootResultRelInfo. Such ResultRelInfos are created when firing RI triggers in tables that are not modified directly. None of the current callers of these functions pass such relations, so this isn't a live bug, but let's make them more robust. Also update comment in ResultRelInfo; after commit 6214e2b, ri_RangeTableIndex is zero for ResultRelInfos created for partition tuple routing. Noted by Coverity. Backpatch down to v11, like commit 6214e2b. Reviewed-by: Tom Lane, Amit Langote
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
@@ -1075,10 +1075,10 @@ Bitmapset *
 ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate)
 {
 	/*
-	 * The columns are stored in the range table entry. If this ResultRelInfo
-	 * doesn't have an entry in the range table (i.e. if it represents a
-	 * partition routing target), fetch the parent's RTE and map the columns
-	 * to the order they are in the partition.
+	 * The columns are stored in the range table entry.  If this ResultRelInfo
+	 * represents a partition routing target, and doesn't have an entry of its
+	 * own in the range table, fetch the parent's RTE and map the columns to
+	 * the order they are in the partition.
 	 */
 	if (relinfo->ri_RangeTableIndex != 0)
 	{
@@ -1087,7 +1087,7 @@ ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate)
 
 		return rte->insertedCols;
 	}
-	else
+	else if (relinfo->ri_RootResultRelInfo)
 	{
 		ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo;
 		RangeTblEntry *rte = rt_fetch(rootRelInfo->ri_RangeTableIndex,
@@ -1102,6 +1102,16 @@ ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate)
 		else
 			return rte->insertedCols;
 	}
+	else
+	{
+		/*
+		 * The relation isn't in the range table and it isn't a partition
+		 * routing target.  This ResultRelInfo must've been created only for
+		 * firing triggers and the relation is not being inserted into.  (See
+		 * ExecGetTriggerResultRel.)
+		 */
+		return NULL;
+	}
 }
 
 /* Return a bitmap representing columns being updated */
@@ -1116,7 +1126,7 @@ ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate)
 
 		return rte->updatedCols;
 	}
-	else
+	else if (relinfo->ri_RootResultRelInfo)
 	{
 		ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo;
 		RangeTblEntry *rte = rt_fetch(rootRelInfo->ri_RangeTableIndex,
@@ -1131,4 +1141,6 @@ ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate)
 		else
 			return rte->updatedCols;
 	}
+	else
+		return NULL;
 }

Original file line number	Diff line number	Diff line change
`@@ -1075,10 +1075,10 @@ Bitmapset *`
`1075`	`1075`	`ExecGetInsertedCols(ResultRelInfo relinfo, EState estate)`
`1076`	`1076`	`{`
`1077`	`1077`	`/*`
`1078`		`- * The columns are stored in the range table entry. If this ResultRelInfo`
`1079`		`- * doesn't have an entry in the range table (i.e. if it represents a`
`1080`		`- * partition routing target), fetch the parent's RTE and map the columns`
`1081`		`- * to the order they are in the partition.`
	`1078`	`+ * The columns are stored in the range table entry. If this ResultRelInfo`
	`1079`	`+ * represents a partition routing target, and doesn't have an entry of its`
	`1080`	`+ * own in the range table, fetch the parent's RTE and map the columns to`
	`1081`	`+ * the order they are in the partition.`
`1082`	`1082`	`*/`
`1083`	`1083`	`if (relinfo->ri_RangeTableIndex != 0)`
`1084`	`1084`	`{`
`@@ -1087,7 +1087,7 @@ ExecGetInsertedCols(ResultRelInfo relinfo, EState estate)`
`1087`	`1087`
`1088`	`1088`	`return rte->insertedCols;`
`1089`	`1089`	`}`
`1090`		`- else`
	`1090`	`+ else if (relinfo->ri_RootResultRelInfo)`
`1091`	`1091`	`{`
`1092`	`1092`	`ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo;`
`1093`	`1093`	`RangeTblEntry *rte = rt_fetch(rootRelInfo->ri_RangeTableIndex,`
`@@ -1102,6 +1102,16 @@ ExecGetInsertedCols(ResultRelInfo relinfo, EState estate)`
`1102`	`1102`	`else`
`1103`	`1103`	`return rte->insertedCols;`
`1104`	`1104`	`}`
	`1105`	`+ else`
	`1106`	`+ {`
	`1107`	`+ /*`
	`1108`	`+ * The relation isn't in the range table and it isn't a partition`
	`1109`	`+ * routing target. This ResultRelInfo must've been created only for`
	`1110`	`+ * firing triggers and the relation is not being inserted into. (See`
	`1111`	`+ * ExecGetTriggerResultRel.)`
	`1112`	`+ */`
	`1113`	`+ return NULL;`
	`1114`	`+ }`
`1105`	`1115`	`}`
`1106`	`1116`
`1107`	`1117`	`/* Return a bitmap representing columns being updated */`
`@@ -1116,7 +1126,7 @@ ExecGetUpdatedCols(ResultRelInfo relinfo, EState estate)`
`1116`	`1126`
`1117`	`1127`	`return rte->updatedCols;`
`1118`	`1128`	`}`
`1119`		`- else`
	`1129`	`+ else if (relinfo->ri_RootResultRelInfo)`
`1120`	`1130`	`{`
`1121`	`1131`	`ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo;`
`1122`	`1132`	`RangeTblEntry *rte = rt_fetch(rootRelInfo->ri_RangeTableIndex,`
`@@ -1131,4 +1141,6 @@ ExecGetUpdatedCols(ResultRelInfo relinfo, EState estate)`
`1131`	`1141`	`else`
`1132`	`1142`	`return rte->updatedCols;`
`1133`	`1143`	`}`
	`1144`	`+ else`
	`1145`	`+ return NULL;`
`1134`	`1146`	`}`