Re: addRangeTableEntry() relies on pstate, contrary to its documentation

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> Off list Tom commented that suggestion with:
>> An easy alternative fix, of course, is to not call isLockedRefname if
>> we don't have a pstate (or else put the pstate==NULL test inside it).

> I'm not a big fan of that - won't that essentially cause the wrong
> locklevel to be used and thus open the door for lock upgrade deadlocks?

Well, it would amount to assuming that the table was not mentioned in
"FOR UPDATE". Depending on context, that might be perfectly appropriate.

A quick grep finds these places that are visibly passing NULL to one or
another addRangeTableEntry* function:

convert_ANY_sublink_to_join(): pulls up an ANY subquery with

rte = addRangeTableEntryForSubquery(NULL, ...

UpdateRangeTableOfViewParse(): inserts NEW/OLD RTEs using

rt_entry1 = addRangeTableEntryForRelation(NULL, viewRel,
makeAlias("old", NIL),
false, false);
rt_entry2 = addRangeTableEntryForRelation(NULL, viewRel,
makeAlias("new", NIL),
false, false);

So you would certainly break these callers. I'm not sure whether any of
the callers that are passing down their own pstate arguments can ever be
passed a NULL; I'm inclined to doubt it though.

An alternative of course is to not have this API spec for all
addRangeTableEntry* functions, but just the two used this way.
I don't much care for that though.

regards, tom lane