Re: revised sample SRF C function; proposed SRF API

Tom Lane wrote:
> Joe Conway <mail(at)joeconway(dot)com> writes:
>
>> If not, prepare an array of C strings representing the attribute
>> values of your return tuple, and call: FUNC_BUILD_SLOT(values,
>> funcctx);
>
> I think that's a poor choice of abstraction, as it forces the user
> into the least-efficient-possible way of building a return tuple.
> What if he's already got a tuple (eg, he read it off disk), or at
> any rate has datums already in internal format? I'd say make it
>
> FUNC_RETURN_NEXT(funcctx, HeapTuple)
>
> and let the caller worry about calling heap_formtuple or otherwise
> constructing the tuple.

Hmmm - well, I agree that FUNC_RETURN_NEXT(funcctx, HeapTuple) is a
better abstraction, particularly for experience backend hackers ;)
but I was trying to also make this accessable to someone writing a
custom C function that isn't necessarily very familiar with forming
their own HeapTuples manually. What if we also had something like:

FUNC_BUILD_TUPLE(values, funcctx);

which returns a tuple for the less experienced folks (or people like me
when I'm being lazy :)) It could be used when desired, or skipped
entirely if a HeapTuple is already easily available.

>
> For similar reasons I think the initial call ought to provide a TupleDesc
> structure, not a relation name (which is at least two lookups removed
> from the information you actually need).

Same comments. How about:
FUNC_BUILD_TUPDESC(_relname)
and
FUNC_MULTIPLE_RESULT(_funcctx, _tupdesc, _max_calls, _fctx)
?

Power hackers could skip FUNC_BUILD_TUPDESC if they wanted to or already
had a TupleDesc available.

Of course you would only want to build your tupdesc during the first
pass, so maybe we'd need
FUNC_IS_FIRSTPASS()
which would just check for (fcinfo->flinfo->fn_extra == NULL)

>
> The max_calls thing doesn't seem quite right either; at least not as
> something that has to be provided in the "first line after the
> function declarations". It might be quite expensive to derive, and
> you don't need to do so on every call.

I thought about that, but the value is not required at all, and you can
easily set it later when more convenient. Perhaps it should be taken out
of the initialization and we just document how it might be used?

> Perhaps better have the macro return a boolean indicating whether
> this is the first call or not, and then people can do
>
> if (FUNC_MULTIPLE_RESULT(funcctx)) { // do one-time setup here, //
> including possibly computing a max_calls value; // also find or make
> a TupleDesc to be stored into the // funcctx. }

hmm - see complete new example below.

>
> Similarly I'm confused about the usefulness of misc_ctx if it has to
> be re-provided on every call.

Like max_calls, maybe it should be taken out of the initialization and
its potential use documented.

On second thought, I think maybe I tried to do too much with
FUNC_MULTIPLE_RESULT. It does initialization during the first pass, and
then does per call setup for subsequent calls. Maybe there should be:

FUNC_FIRSTCALL_INIT
and
FUNC_PERCALL_SETUP

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);

/*
* 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)

/*
* Here we need to test whether or not we're all out
* of tuples to return. The test does not have to be
* this one, but in many cases this is probably what
* you'll want.
*/
if (call_cntr < max_calls)
{
/*
* user code to derive data to be returned
*/
<user defined code>

/*
* Optional - build a HeapTuple given user data
* in C string form
* values is an array of C strings, one for each
* attribute of the return tuple
*/
tuple = FUNC_BUILD_TUPLE(values, funcctx);

/*
* Required - returns the tuple and notifies
* the caller that we still have more to do
*/
FUNC_RETURN_NEXT(funcctx, HeapTuple);
}
else
{
/*
* Required - returns NULL tuple and notifies
* caller that we're all done now.
*/
FUNC_RETURN_DONE(funcctx);
}
}

How's this look? Any better?

Thanks,

Joe