Re: Last call for comments: fmgr rewrite [LONG] - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Last call for comments: fmgr rewrite [LONG]
Date
Msg-id 17962.959009832@sss.pgh.pa.us
Whole thread Raw
In response to Last call for comments: fmgr rewrite [LONG]  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
JanWieck@t-online.de (Jan Wieck) writes:
>     I'm  not  totally  sure  what  you  mean  with the ugly, non-
>     reentrant   kluge.    I    assume    it's    this    annoying
>     setjmp()/longjmp() juggling - isn't it?

No, I was unhappy with the global variables like fmgr_pl_info and
CurrentTriggerData.  As you say, error handling in the PL managers
is pretty ugly, but I don't see a way around that --- and at least
the ugliness is localized ;-)

>     A new querytree structure cannot gain  it,  if  the  function
>     manager  cannot  handle  it.  At  least we need to define how
>     tuple sets as arguments and results should be handled in  the
>     future,  and  define  the  fmgr  interface  according to that
>     already.

At the moment I'm satisfied to have a trapdoor that allows extension of
the fmgr interface --- that's what the context and resultinfo fields are
intended for.  In my mind this is a limited redesign of one specific API
for limited objectives.  If we try to turn the project into "fix
everything anyone could possibly want for functions" then nothing will
get done at all...

>> resultinfo is NULL when calling any function from which a simple Datum
>> result is expected.  It may point to some subtype of Node if the function
>> returns more than a Datum.  Like the context field, resultinfo is a hook
>> for expansion; fmgr itself doesn't constrain the use of the field.

>     Good  place  to  put  in  a  tuple descriptor for [SET] tuple
>     return types.  But the same type  of  information  should  be
>     there per argument.

The context field could be used to pass additional information about
arguments, too.  Actually, the way things are currently coded, it
wouldn't be hard to throw in more extension pointers like context
and resultinfo, so long as they are defined to default to NULL for
simple calls of functions accepting and returning Datums.  As I was
remarking to Chris, I have some concern about not bloating the struct,
but a pointer or two more or less won't hurt.

>     At  this  point I'd like to add another relkind we might want
>     to have.  This relkind  just  describes  a  tuple  structure,
>     without having a heap or rules. Only to define a complex type
>     to be used in function declarations.

Could be a good idea.  In the original Postgres code it seems the only
way to define a tuple type is to create a table with that structure
--- but maybe you have no intention of using the table, and only want
the type...

>> 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[].

>     If you already think about calling  the  same  function  with
>     variable number of arguments, where are the argtypes?

Not fmgr's problem --- it doesn't know a thing about the argument or
result types.  I'm not sure that the variable-arguments business will
ever really get implemented; I just wanted to be sure that these data
structures could represent it if we do want to implement it.

>> For TOAST-able data types, the PG_GETARG macro will deliver a de-TOASTed
>> data value.  There might be a few cases where the still-toasted value is
>> wanted, but I am having a hard time coming up with examples.

>     length()   and   octetlength()  are  good  candidates.

OK, so it will be possible to get at the still-toasted value.

>     For the two PL handlers I wrote that's enough.  They  allways
>     store  their  own  private  information  in their own private
>     memory. Having some place there which is initialized to NULL,
>     where  they  can  leave  a  pointer to avoid a lookup at each
>     invocation is perfect.

Yes, I've already changed them to do this ;-).

>> In the initial phase, two new entries will be added to pg_language
>> for language types "newinternal" and "newC", corresponding to
>> builtin and dynamically-loaded functions having the new calling
>> convention.

>     I would prefer "interal_ext" and "C_ext".

Someone else suggested renaming the old languages types to "oldXXX"
and giving the new ones pride of place with the basic names "internal"
and "C".  For the internal functions we could do this if we like.
For dynamically loaded functions we will break existing code (or at
least the CREATE FUNCTION scripts for it) if we don't stick with "C"
as the name for the old-style interface.  Is that worth the long-term
niceness of a simple name for the new-style interface?  I went for
compatibility but I won't defend it very hard.  Comments anyone?

>     What I'm missing (don't know  which  of  these  are  standard
>     compliant):

>         Extending  the  system  catalog to give arguments a name.

>         Extending  the  system  catalog to provide default values
>         for arguments.

>         Extending call semantics so  functions  can  have  INPUT,
>         OUTPUT and INOUT arguments.

None of these are fmgr's problem AFAICS, nor do I see a reason to
add them to the current work proposal.  They look like a future
project to me...
        regards, tom lane


pgsql-hackers by date:

Previous
From: Chris Bitmead
Date:
Subject: SQL3 UNDER
Next
From: "Ross J. Reedstrom"
Date:
Subject: Re: A test to add to the crashme test