At Wed, 10 Feb 2021 19:54:46 -0300, Ranier Vilela <ranier.vf@gmail.com> wrote in > Hi, > > Per Coverity. > > The functions ExecGetInsertedCols and ExecGetUpdatedCols at ExecUtils.c > only are safe to call if the variable "ri_RangeTableIndex" is != 0. > > Otherwise a possible Dereference after null check (FORWARD_NULL) can be > raised.
As it turns out, it's a false positive. And perhaps we don't want to take action just to satisfy the static code analyzer.
The coment in ExecGetInsertedCols says:
> /* > * 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. > */ > if (relinfo->ri_RangeTableIndex != 0) > {
This means that any one of the two is always usable here. AFAICS, actually, ri_RangeTableIndex is non-zero for partitioned (=leaf) and non-partitoned relations and ri_RootResultRelInfo is non-null for partitioned (parent or intermediate) relations (since they don't have a coressponding range table entry).
The only cases where both are 0 and NULL are trigger-use, which is unrelated to the code path.
This is a case where it would be worth an assertion.
What do you think?
Apparently my efforts with Coverity have been worth it. And together we are helping to keep Postgres more secure. Although sometimes it is not recognized for that [1].
