On 08.06.21 01:10, Tom Lane wrote:
> I wrote:
>> Hmm, these are atop HEAD from a week or so back. The cfbot seems to
>> think they still apply. In any case, I was about to spend some effort
>> on the docs, so I'll post an updated version soon (hopefully today).
>
> Here is said update (rolled up into one patch this time; maybe that will
> avoid the apply problems you had).
This patch looks good to me.
A minor comment: You changed the docs in some places like this:
- </itemizedlist></para>
+ </itemizedlist>
+ </para>
The original layout is required to avoid spurious whitespace in the
output (mainly affecting man pages).
> I noticed that there is one other loose end in the patch: should
> LookupFuncName() really be passing OBJECT_ROUTINE to
> LookupFuncNameInternal()? This matches its old behavior, in which
> no particular routine type restriction was applied; but I think that
> the callers are nearly all expecting that only plain functions will
> be returned. That's more of a possible pre-existing bug than it
> is the fault of the patch, but nonetheless this might be a good
> time to resolve it.
It appears that all uses of LookupFuncName() are lookups of internal
support functions (with one exception in pltcl), so using
OBJECT_FUNCTION would be okay.
It might be good to keep the argument order of LookupFuncNameInternal()
consistent with LookupFuncWithArgs() with respect to the new ObjectType
argument.