Re: revised sample SRF C function; proposed SRF API

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: revised sample SRF C function; proposed SRF API
Date: 2002-05-28 14:50:48
Message-ID: 3051.1022597448@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Joe Conway <mail(at)joeconway(dot)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
not
HeapTuple 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Sullivan 2002-05-28 14:53:46 Re: [HACKERS] Re : Solaris Performance - 64 bit puzzle
Previous Message Andrew Sullivan 2002-05-28 14:50:25 Re: Replication status & point-in-time recovery

Browse pgsql-patches by date

  From Date Subject
Next Message Peter Eisentraut 2002-05-28 16:53:34 Re: SSL (patch 1)
Previous Message Valentine Zaretsky 2002-05-28 12:48:52 Re: SRF rescan testing