Skip to content

Commit

Permalink
Make ExecGetInsertedCols() and friends more robust and improve comments.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
hlinnaka committed Feb 15, 2021
1 parent 46d6e5f commit 54e51dc
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 13 deletions.
28 changes: 21 additions & 7 deletions src/backend/executor/execUtils.c
Original file line number Diff line number Diff line change
Expand Up @@ -1230,18 +1230,18 @@ 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)
{
RangeTblEntry *rte = exec_rt_fetch(relinfo->ri_RangeTableIndex, estate);

return rte->insertedCols;
}
else
else if (relinfo->ri_RootResultRelInfo)
{
ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo;
RangeTblEntry *rte = exec_rt_fetch(rootRelInfo->ri_RangeTableIndex, estate);
Expand All @@ -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 */
Expand All @@ -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);
Expand All @@ -1276,6 +1286,8 @@ ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate)
else
return rte->updatedCols;
}
else
return NULL;
}

/* Return a bitmap representing generated columns being updated */
Expand All @@ -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);
Expand All @@ -1300,6 +1312,8 @@ ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate)
else
return rte->extraUpdatedCols;
}
else
return NULL;
}

/* Return columns being updated, including generated columns */
Expand Down
15 changes: 9 additions & 6 deletions src/include/nodes/execnodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down

0 comments on commit 54e51dc

Please sign in to comment.