From 54e51dcde03e5c746e8de6243c69fafdc8d0ec7a Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Mon, 15 Feb 2021 09:28:08 +0200
Subject: [PATCH] Make ExecGetInsertedCols() and friends more robust and
improve comments.
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 6214e2b228,
ri_RangeTableIndex is zero for ResultRelInfos created for partition tuple
routing.
Noted by Coverity. Backpatch down to v11, like commit 6214e2b228.
Reviewed-by: Tom Lane, Amit Langote
---
src/backend/executor/execUtils.c | 28 +++++++++++++++++++++-------
src/include/nodes/execnodes.h | 15 +++++++++------
2 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index c734283bfe9..42632cb4d88 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -1230,10 +1230,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)
{
@@ -1241,7 +1241,7 @@ ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate)
return rte->insertedCols;
}
- else
+ else if (relinfo->ri_RootResultRelInfo)
{
ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo;
RangeTblEntry *rte = exec_rt_fetch(rootRelInfo->ri_RangeTableIndex, estate);
@@ -1252,6 +1252,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 */
@@ -1265,7 +1275,7 @@ ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate)
return rte->updatedCols;
}
- else
+ else if (relinfo->ri_RootResultRelInfo)
{
ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo;
RangeTblEntry *rte = exec_rt_fetch(rootRelInfo->ri_RangeTableIndex, estate);
@@ -1276,6 +1286,8 @@ ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate)
else
return rte->updatedCols;
}
+ else
+ return NULL;
}
/* Return a bitmap representing generated columns being updated */
@@ -1289,7 +1301,7 @@ ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate)
return rte->extraUpdatedCols;
}
- else
+ else if (relinfo->ri_RootResultRelInfo)
{
ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo;
RangeTblEntry *rte = exec_rt_fetch(rootRelInfo->ri_RangeTableIndex, estate);
@@ -1300,6 +1312,8 @@ ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate)
else
return rte->extraUpdatedCols;
}
+ else
+ return NULL;
}
/* Return columns being updated, including generated columns */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index b6a88ff76b8..943931f65d0 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -394,12 +394,15 @@ typedef struct OnConflictSetState
* relation, and perhaps also fire triggers. ResultRelInfo holds all the
* information needed about a result relation, including indexes.
*
- * Normally, a ResultRelInfo refers to a table that is in the query's
- * range table; then ri_RangeTableIndex is the RT index and ri_RelationDesc
- * is just a copy of the relevant es_relations[] entry. But sometimes,
- * in ResultRelInfos used only for triggers, ri_RangeTableIndex is zero
- * and ri_RelationDesc is a separately-opened relcache pointer that needs
- * to be separately closed. See ExecGetTriggerResultRel.
+ * Normally, a ResultRelInfo refers to a table that is in the query's range
+ * table; then ri_RangeTableIndex is the RT index and ri_RelationDesc is
+ * just a copy of the relevant es_relations[] entry. However, in some
+ * situations we create ResultRelInfos for relations that are not in the
+ * range table, namely for targets of tuple routing in a partitioned table,
+ * and when firing triggers in tables other than the target tables (See
+ * ExecGetTriggerResultRel). In these situations, ri_RangeTableIndex is 0
+ * and ri_RelationDesc is a separately-opened relcache pointer that needs to
+ * be separately closed.
*/
typedef struct ResultRelInfo
{