NULL-pointer check and incorrect comment for pstate in addRangeTableEntry

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