Re: [rfc] new CREATE FUNCTION (and more) - Mailing list pgsql-hackers

From Philip Warner
Subject Re: [rfc] new CREATE FUNCTION (and more)
Date
Msg-id 3.0.5.32.20001117144956.02c51570@mail.rhyme.com.au
Whole thread Raw
In response to Re: [rfc] new CREATE FUNCTION (and more)  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [rfc] new CREATE FUNCTION (and more)  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
At 22:10 16/11/00 -0500, Tom Lane wrote:
>Philip Warner <pjw@rhyme.com.au> writes:
>> For possible future compatibility, can you also do something like:
>>     PG_FUNCTION_API_V2;
>>     PG_FUNCTION_V2(foo);
>>     PG_FUNCTION_V2(bar);
>>     ...
>
>> Where
>> PG_FUNCTION_API_V2 expands to:
>>     int pg_fmgr_api_version(void) { return 2; }
>> And PG_FUNCTION_V2(foo) either does nothing or expands to:
>>     int pg_fmgr_api2_version_foo(void) { return 2; }
>
>I'm not following the point here.  Why two different macros?  It doesn't
>look to me like the first one does anything.  The per-routine macro
>calls should be capable of doing everything that needs to be done.

I think the PG_FUNCTION_API_V2 macros is very important because it will
tell the function manager which 'protocol' to use. 

In my view, the individual stub entry points for each function are part of
the protocol and should not be assumed. Returning a struct would be ideal
since it allows more flexibility in the furture.

So long as the version is always in the first bytes of the struct, we are
covered for compatibility.


>> The first call will tell PG that (because it is version 2), it should
>> expect the next set of entry points. Since we will not be allowing mixed
>> versions in this version of the API (I think),
>
>Yes, we will, because there is a case in the regression tests that
>will break anything that doesn't cope with mixed versions ;-).
>I deliberately left some of the routines in regress.c old-style ...

I'd still argue for a PG_FUNCTION_API_V2 macro for the reasons above. What
the fmgrs needs to do is:

- call pg_fmgr_api_version() to get the protocol version
- when it wants to call a function 'foo' see if there is a 'pg_api_foo'
entry point, and if so, use the new interface, o/wise use the old one. No
need to even call it.

Future versions will call pg_fmgr_api_version() and possibly pass
appropriate structs to info-functions or whatever.


>
>Hmm.  This stub idea might be a sufficient reason to say that we want to
>do a function call rather than look for a global variable.

I agree.


>I am unpersuaded by the idea that a one-liner function per useful entry
>point is an intolerable amount of overhead.  Let's keep it simple here.

Wasn't worried about the overhead at all, just the offense to my aesthetics
8-).


>>     PG_FUNCTION_V3(foo, false, true, foo_entry_point)
>> expand to:
>>     void pg_fmgr_api_version_foo(fmgr_info *i) 
>>        { i->iscacheable=false; 
>i-> isstrict=true;
>i-> entrypoint=foo_entry_point; }
>
>I prefer something like
>
>    const inforec * pg_api_foo(void)
>    {
>        static inforec foo_info = { ... };
>        return &foo_info;
>    }
>
>since this avoids prejudging anything.  (In your example, how does
>the version function *know* how big the record it's been handed is?

Becuase the function manager called pg_fmgr_api_version and made sure it
passed the right structure.


>Loading a version-N library into a Postgres version < N might bomb
>hard because the info function scribbles on fields that aren't there.

If it calls pg_fmgr_api_version and can't get an acceptable version, it
would bomb nicely. Maybe in future versions we could even support some kind
of protocol negotiation, but that seems less than useful at this stage.


>Handing back a pointer to something that the main code then treats
>as read-only seems much safer.)

This is fine too; but in all cases they have to be sure that they agree on
what is being handed back. So long at the blocks always have the version
number on the header it is fine.

>> Perhaps in PG_FUNCTION_API_V4 we can implement some kind of interface for
>> listing supported entry points for module loading...
>
>I think that should be seen as a separate feature, rather than
>mixing it up with support information about any individual function.

Fine.


My only real issue with all of this is that we need to separate the
protocol selection from the the data exchange.



----------------------------------------------------------------
Philip Warner                    |     __---_____
Albatross Consulting Pty. Ltd.   |----/       -  \
(A.B.N. 75 008 659 498)          |          /(@)   ______---_
Tel: (+61) 0500 83 82 81         |                 _________  \
Fax: (+61) 0500 83 82 82         |                 ___________ |
Http://www.rhyme.com.au          |                /           \|                                |    --________--
PGP key available upon request,  |  /
and from pgp5.ai.mit.edu:11371   |/


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [rfc] new CREATE FUNCTION (and more)
Next
From: Tom Lane
Date:
Subject: Re: [rfc] new CREATE FUNCTION (and more)