Hi all,
Coverity is pointing out that addRangeTableEntry contains the
following code path that does a NULL-pointer check on pstate:
if (pstate != NULL)
pstate->p_rtable = lappend(pstate->p_rtable, rte);
But pstate is dereferenced before in isLockedRefname when grabbing the
lock mode:
lockmode = isLockedRefname(pstate, refname) ? RowShareLock : AccessShareLock;
Note that there is as well the following comment that is confusing on
top of addRangeTableEntry:
* If pstate is NULL, we just build an RTE and return it without adding it
* to an rtable list.
So I have looked at all the code paths calling this function and found
first that the only potential point where pstate could be NULL is
transformTopLevelStmt in analyze.c. One level higher there are
parse_analyze_varparams and parse_analyze that may use pstate as NULL,
and even one level more higher in the stack there is
pg_analyze_and_rewrite. But well, looking at each case individually,
in all cases we never pass NULL for the parse tree node, so I think
that we should remove the comment on top of addRangeTableEntry, remove
the NULL-pointer check and add an assertion as in the patch attached.
Thoughts?
Regards,
--
Michael
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 8d4f79f..f416fc2 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -993,9 +993,6 @@ parserOpenTable(ParseState *pstate, const RangeVar *relation, int lockmode)
/*
* Add an entry for a relation to the pstate's range table (p_rtable).
*
- * If pstate is NULL, we just build an RTE and return it without adding it
- * to an rtable list.
- *
* Note: formerly this checked for refname conflicts, but that's wrong.
* Caller is responsible for checking for conflicts in the appropriate scope.
*/
@@ -1011,6 +1008,8 @@ addRangeTableEntry(ParseState *pstate,
LOCKMODE lockmode;
Relation rel;
+ Assert(pstate != NULL);
+
rte->rtekind = RTE_RELATION;
rte->alias = alias;
@@ -1058,8 +1057,7 @@ addRangeTableEntry(ParseState *pstate,
* Add completed RTE to pstate's range table list, but not to join list
* nor namespace --- caller must do that if appropriate.
*/
- if (pstate != NULL)
- pstate->p_rtable = lappend(pstate->p_rtable, rte);
+ pstate->p_rtable = lappend(pstate->p_rtable, rte);
return rte;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
