Re: revised sample SRF C function; proposed SRF API - Mailing list pgsql-hackers

Joe Conway <mail@joeconway.com> writes:
> What if we also had something like:
>     FUNC_BUILD_TUPLE(values, funcctx);
> which returns a tuple for the less experienced folks

Sure, as long as it's not getting in the way when you don't want it.
For that matter the FUNC stuff shouldn't get in the way of using it
in other contexts, so I'd suggest decoupling it from funcctx.  Why
notHeapTuple BuildTupleFromStrings(TupDesc, char**)
(better choice of name welcome).


> Then the whole API looks something like:

> Datum
> my_Set_Returning_Function(PG_FUNCTION_ARGS)
> {
>     FuncCallContext *funcctx;
>     <user defined declarations>

>     /*
>      * Optional - user defined code needed to be called
>      * on every pass
>      */
>     <user defined code>

>     if(FUNC_IS_FIRSTPASS())
>     {
>        /*
>         * Optional - user defined initialization which is only
>         * required during the first pass through the function
>         */
>        <user defined code>

>        /*
>         * Optional - if desired, use this to get a TupleDesc
>         * based on the function's return type relation
>         */
>        FUNC_BUILD_TUPDESC(_relname);

>        /*
>         * Required - memory allocation and initialization
>         * which is only required during the first pass through
>         * the function
>         */
>        FUNC_FIRSTCALL_INIT(funcctx, tupdesc);

I think this should be
funcctx = FUNC_FIRSTCALL_INIT(tupdesc);

to make it clearer that it is initializing funcctx.  Similarly
FUNC_BUILD_TUPDESC should be more like
tupdesc = RelationNameGetTupleDesc(relname);

since it's not particularly tied to this usage.

>        /*
>         * optional - total number of tuples to be returned.
>         *
>         */
>        funcctx->max_calls = my_max_calls;

>        /*
>         * optional - pointer to structure containing
>         * user defined context
>         */
>        funcctx->fctx = my_func_context_pointer;
>     }

>     /*
>      * Required - per call setup
>      */
>     FUNC_PERCALL_SETUP(funcctx)

Again I'd prefer
funcctx = FUNC_PERCALL_SETUP();

I think this is easier for both humans and compilers to recognize
as an initialization of funcctx.

> How's this look? Any better?

Definitely better.  I'd suggest also thinking about whether the
same/similar macros can support functions that return a set of a
scalar (non-tuple) datatype.  In my mind, the cleanest design would
be some base macros that support functions-returning-set (of anything),
and if you want to return a set of scalar then you just use these
directly (handing a Datum to FUNC_RETURN_NEXT).  If you want to return
a set of tuples then there are a couple extra steps that you need to
do to build a tupdesc, build a tuple, and convert the tuple to Datum
(which at the moment you do by putting it into a slot, but I think we
ought to change that soon).  If it were really clean then the macros
supporting these extra steps would also work without the SRF macros,
so that you could use 'em in a function returning a single tuple.
        regards, tom lane