Function-manager redesign: second draft (long) - Mailing list pgsql-hackers

From Tom Lane
Subject Function-manager redesign: second draft (long)
Date
Msg-id 2395.940722749@sss.pgh.pa.us
Whole thread Raw
Responses Re: [HACKERS] Function-manager redesign: second draft (long)
Re: [HACKERS] Function-manager redesign: second draft (long)
List pgsql-hackers
This is a followup to a message I wrote in June about reworking the fmgr
interface.  I've had a couple of good ideas (I think ;-)) since then,
but there are some parts of the proposal that still need work before
implementation can begin.

I could particularly use some feedback from Jan and anyone else who's
worked with function-call handlers: does this design eliminate the
kluges that you've had to use in the past?  If not, what else is needed?
        regards, tom lane


Proposal for function-manager redesign
--------------------------------------

We know that the existing mechanism for calling Postgres functions needs
to be redesigned.  It has portability problems because it makes
assumptions about parameter passing that violate ANSI C; it fails to
handle NULL arguments and results cleanly; and "function handlers" that
support a class of functions (such as fmgr_pl) can only be done via a
really ugly, non-reentrant kluge.  (Global variable set during every
function call, forsooth.)  Here is a proposal for fixing these problems.

In the past, the major objections to redoing the function-manager
interface have been (a) it'll be quite tedious to implement, since every
built-in function and everyplace that calls such functions will need to
be touched; (b) such wide-ranging changes will be difficult to make in
parallel with other development work; (c) it will break existing
user-written loadable modules that define "C language" functions.  While
I have no solution to the "tedium" aspect, I believe I see an answer to
the other problems: by use of function handlers, we can support both old
and new interfaces in parallel for both callers and callees, at some
small efficiency cost for the old styles.  That way, most of the changes
can be done on an incremental file-by-file basis --- we won't need a
"big bang" where everything changes at once.  Support for callees
written in the old style can be left in place indefinitely, to provide
backward compatibility for user-written C functions.


The new function-manager interface
----------------------------------

The core of the new design is revised data structures for representing
the result of a function lookup and for representing the parameters
passed to a specific function invocation.  (We want to keep function
lookup separate, since many parts of the system apply the same function
over and over; the lookup overhead should be paid once per query, not
once per tuple.)


When a function is looked up in pg_proc, the result is represented as

typedef struct
{   PGFunction  fn_addr;    /* pointer to function or handler to be called */   Oid         fn_oid;     /* OID of
function(NOT of handler, if any) */   int         fn_nargs;   /* 0..MAXFMGRARGS, or -1 if variable arg count */   void
    *fn_extra;   /* extra space for use by handler */
 
} FunctionLookupInfoData;
typedef FunctionLookupInfoData* FunctionLookupInfo;

For an ordinary built-in function, fn_addr is just the address of the C
routine that implements the function.  Otherwise it is the address of a
handler for the class of functions that includes the target function.
The handler can use the function OID and perhaps also the fn_extra slot
to find the specific code to execute.  (fn_oid = InvalidOid can be used
to denote a not-yet-initialized FunctionLookupInfoData struct.  fn_extra
will always be NULL when a FunctionLookupInfoData is first filled by the
function lookup code, but a function handler could set it to avoid
making repeated lookups of its own when the same FunctionLookupInfoData
is used repeatedly during a query.)  fn_nargs is the number of arguments
expected by the function.

FunctionLookupInfo replaces the present FmgrInfo structure (but I'm
giving it a new name so that the old struct definition can continue
to exist during the transition phase).


During a call of a function, the following data structure is created
and passed to the function:

typedef struct
{   FunctionLookupInfo flinfo;  /* ptr to lookup info used for this call */   bool        isnull;         /*
input/outputflag, see below */   int         nargs;          /* # arguments actually passed */   Datum
arg[MAXFMGRARGS]; /* Arguments passed to function */   bool        argnull[MAXFMGRARGS];  /* T if arg[i] is actually
NULL*/
 
} FunctionCallInfoData;
typedef FunctionCallInfoData* FunctionCallInfo;

Note that all the arguments passed to a function (as well as its result
value) will now uniformly be of type Datum.  As discussed below, callers
and callees should apply the standard Datum-to-and-from-whatever macros
to convert to the actual argument types of a particular function.  The
value in arg[i] is unspecified when argnull[i] is true.

It is generally the responsibility of the caller to ensure that the
number of arguments passed matches what the callee is expecting: except
for callees that take a variable number of arguments, the callee will
typically ignore the nargs field and just grab values from arg[].

The meaning of the struct elements should be pretty obvious with the
exception of isnull.  isnull must be set by the caller to the logical OR
of the argnull[i] flags --- ie, isnull is true if any argument is NULL.
(Of course, isnull is false if nargs == 0.)  On return from the
function, isnull is the null flag for the function result: if it is true
the function's result is NULL, regardless of the actual function return
value.  Overlapping the input and output flags in this way provides a
simple, convenient, fast implementation for the most common case of a
"strict" function (whose result is NULL if any input is NULL):
if (finfo->isnull)    return (Datum) 0;   /* specific value doesn't matter */
... else do normal calculation ignoring argnull[] ...

Non-strict functions can easily be implemented; they just need to check
the individual argnull[] flags and set the appropriate isnull value
before returning.

FunctionCallInfo replaces FmgrValues plus a bunch of ad-hoc parameter
conventions.


Callees, whether they be individual functions or function handlers,
shall always have this signature:

Datum function (FunctionCallInfo finfo);

which is represented by the typedef

typedef Datum (*PGFunction) (FunctionCallInfo finfo);

The function is responsible for setting finfo->isnull appropriately
as well as returning a result represented as a Datum.  Note that since
all callees will now have exactly the same signature, and will be called
through a function pointer declared with exactly that signature, we
should have no portability or optimization problems.

When the function's result type is pass-by-reference, the result value
must always be stored in freshly-palloc'd space (it can't be a constant
or a copy of an input pointer).  This rule will eventually allow
automatic reclamation of storage space during expression evaluation.


Function coding conventions
---------------------------

As an example, int4 addition goes from old-style

int32
int4pl(int32 arg1, int32 arg2)
{   return arg1 + arg2;
}

to new-style

Datum
int4pl(FunctionCallInfo finfo)
{   if (finfo->isnull)       return (Datum) 0;        /* value doesn't really matter ... */   /* we can ignore flinfo,
nargsand argnull */
 
   return Int32GetDatum(DatumGetInt32(finfo->arg[0]) +                        DatumGetInt32(finfo->arg[1]));
}

This is, of course, much uglier than the old-style code, but we can
improve matters with some well-chosen macros for the boilerplate parts.
What we actually end up writing might look something like

Datum
int4pl(PG_FUNCTION_ARGS)
{   PG_STRICT_FUNCTION;            /* encapsulates null check */   {       PG_ARG1_INT32;       PG_ARG2_INT32;
       PG_RESULT_INT32( arg1 + arg2 );   }
}

where the macros expand to things like
"int32 arg1 = DatumGetInt32(finfo->arg[0])"
and "return Int32GetDatum( x )".  I don't yet have a detailed proposal
for convenience macros for function authors, but I think it'd be well
worth while to define some.

For the standard pass-by-reference types (int8, float4, float8) these
macros should also hide the indirection and space allocation involved,
so that the function's code is not explicitly aware that the types are
pass-by-ref.  This will allow future conversion of these types to
pass-by-value on machines where it's feasible to do that.  (For example,
on an Alpha it's pretty silly to make int8 be pass-by-ref, since Datum
is going to be 64 bits anyway.)


Call-site coding conventions
----------------------------

There are many places in the system that call either a specific function
(for example, the parser invokes "textin" by name in places) or a
particular group of functions that have a common argument list (for
example, the optimizer invokes selectivity estimation functions with
a fixed argument list).  These places will need to change, but we should
try to avoid making them significantly uglier than before.

Places that invoke an arbitrary function with an arbitrary argument list
can simply be changed to fill a FunctionCallInfoData structure directly;
that'll be no worse and possibly cleaner than what they do now.

When invoking a specific built-in function by name, we have generally
just written something likeresult = textin ( ... args ... )
which will not work after textin() is converted to the new call style.
I suggest that code like this be converted to use "helper" functions
that will create and fill in a FunctionCallInfoData struct.  For
example, if textin is being called with one argument, it'd look
something likeresult = DirectFunctionCall1(textin, PointerGetDatum(argument));
These helper routines will have declarations likeDatum DirectFunctionCall2(PGFunction func, Datum arg1, Datum arg2);
Note it will be the caller's responsibility to convert to and from
Datum; appropriate conversion macros should be used.

The DirectFunctionCallN routines will not bother to fill in
finfo->flinfo (indeed cannot, since they have no idea about an OID for
the target function); they will just set it NULL.  This is unlikely to
bother any built-in function that could be called this way.  Note also
that this style of coding cannot check for a NULL result (it couldn't
before, either!).  We could reasonably make the helper routines elog an
error if they see that the function returns a NULL.

(Note: direct calls like this will have to be changed at the same time
that the called routines are changed to the new style.  But that will
still be a lot less of a constraint than a "big bang" conversion.)

When invoking a function that has a known argument signature, we have
usually written eitherresult = fmgr(targetfuncOid, ... args ... );
orresult = fmgr_ptr(FmgrInfo *finfo, ... args ... );
depending on whether an FmgrInfo lookup has been done yet or not.
This kind of code can be recast using helper routines, in the same
style as above:result = OidFunctionCall1(funcOid, PointerGetDatum(argument));result = FunctionCall2(funcCallInfo,
               PointerGetDatum(argument),                       Int32GetDatum(argument));
 

Again, this style of coding does not recognize the possibility of a
null result.  We could provide variant helper routines that allow
a null return rather than raising an error, which could be called in
a style likeif (FunctionCall1IsNull(&result, funcCallInfo,                        PointerGetDatum(argument))){    ...
copewith null result ...}else{    ... OK, use 'result' here ...}
 
But I'm unsure that there are enough places in the system that need this
to justify the extra set of helpers.  If there are only a few places
that need a non-error response to a null result, they could just be
changed to fill and examine a FunctionCallInfoData structure directly.

As with the callee-side situation, I am strongly inclined to add
argument conversion macros that hide the pass-by-reference nature of
int8, float4, and float8, with an eye to making those types relatively
painless to convert to pass-by-value.

The existing helper functions fmgr(), fmgr_c(), etc will be left in
place until all uses of them are gone.  Of course their internals will
have to change in the first step of implementation, but they can
continue to support the same external appearance.


Notes about function handlers
-----------------------------

Handlers for classes of functions should find life much easier and
cleaner in this design.  The OID of the called function is directly
reachable from the passed parameters; we don't need the global variable
fmgr_pl_finfo anymore.  Also, by modifying finfo->flinfo->fn_extra,
the handler can cache lookup info to avoid repeat lookups when the same
function is invoked many times.  (fn_extra can only be used as a hint,
since callers are not required to re-use a FunctionLookupInfo struct.
But in performance-critical paths they normally will do so.)

I observe that at least one other global variable, CurrentTriggerData,
is being used as part of the call convention for some function handlers.
That's just as grotty as fmgr_pl_finfo, so I'd like to get rid of it.
Any comments on the cleanest way to do so?

Are there any other things needed by the call handlers for PL/pgsql and
other languages?

During the conversion process, support for old-style builtin functions
and old-style user-written C functions will be provided by appropriate
function handlers.  For example, the handler for old-style builtins
will look roughly like fmgr_c() does now.


System table updates
--------------------

In the initial phase, pg_language type 11 ("builtin") will be renamed
to "old_builtin", and a new language type named "builtin" will be
created with a new OID.  Then pg_proc entries will be changed from
language code 11 to the new code piecemeal, as the associated routines
are rewritten.  (This will imply several rounds of forced initdbs as
the contents of pg_proc change.  It would be a good idea to add a
"catalog contents version number" to the database version info checked
at startup before we begin this process.)

The existing pg_language entry for "C" functions will continue to
describe user functions coded in the old style, and we will need to add
a new language name for user functions coded in the new style.  (Any
suggestions for what the new name should be?)  We should deprecate
old-style functions because of their portability problems, but the
support for them will only be one small function handler routine,
so we can leave them in place for as long as necessary.

The expected calling convention for PL call handlers will need to change
all-at-once, but fortunately there are not very many of them to fix.


pgsql-hackers by date:

Previous
From: "Hiroshi Inoue"
Date:
Subject: System indexes are never unique indexes( was RE: [HACKERS] mdnblocks is an amazing time sink in huge relations)
Next
From: Tim Holloway
Date:
Subject: Re: [HACKERS] RFC: Industrial-strength logging (long message)