revised sample SRF C function; proposed SRF API

Joe Conway wrote:

Here is a revised patch for a sample C function returning setof
composite. (Same comments as last time -- It is a clone of SHOW ALL as
an SRF. For the moment, the function is implemented as contrib/showguc,
although a few minor changes to guc.c and guc.h were required to support
it.)

This version includes pieces that may be appropriate for fmgr.c and
fmgr.h, to hide some of the ugliness and facilitate writing C functions
which return setof composite. The API is something like this:

Sorry -- I was a bit too quick with the last patch :-(. It generates a
"Cache reference leak" warning. This one is better.

Joe

Joe Conway <mail@joeconway.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.

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

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. 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.
}

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

regards, tom lane

Joe Conway writes:

Here is a revised patch for a sample C function returning setof
composite. (Same comments as last time -- It is a clone of SHOW ALL as
an SRF. For the moment, the function is implemented as contrib/showguc,
although a few minor changes to guc.c and guc.h were required to support
it.)

We need a function like this in the main line. The "show all" variety
isn't top priority, but we need something that gets you the "show" result
as a query output. The original idea was to make SHOW return a query
result directly, but a function is fine with me too.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut wrote:

We need a function like this in the main line. The "show all" variety
isn't top priority, but we need something that gets you the "show" result
as a query output. The original idea was to make SHOW return a query
result directly, but a function is fine with me too.

Originally I wrote this as "showvars(varname)" and accepted 'all' in a
similar fashion to SHOW ALL. But it seemed redundant since you can still do:

test=# select * from showvars() where varname = 'wal_sync_method';
varname | varval
-----------------+-----------
wal_sync_method | fdatasync
(1 row)

but you can also do:

test=# select * from showvars() where varname like 'show%';
varname | varval
---------------------+--------
show_executor_stats | off
show_parser_stats | off
show_planner_stats | off
show_query_stats | off
show_source_port | off
(5 rows)

which also seemed useful.

I was thinking that if we wanted to replace SHOW X with this, it could
be done in the parser by rewriting it as "SELECT * FROM showvars() WHERE
varname = 'X'", or for SHOW ALL just "SELECT * FROM showvars()".

In any case, I'll fit the showvars() function into the backend and
submit a patch.

Thanks,

Joe

Tom Lane wrote:

Joe Conway <mail@joeconway.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

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

Tom Lane wrote:

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.

Sorry for the long delay. I just got back to this today, and I've run
into an interesting question.

I have a proposal and patch almost ready which I think pretty much meets
the above design requirements. I also wanted to incorporate a built-in
function for returning guc variables (varname text, varval text),
consistent with previous posts. This is both useful and a good test of
the Composite & SRF function API (the API includes functions/macros to
facilitate returning composite types, and an independent set of
functions/macros for returning sets, whether composite or scalar).

The question is how to best bootstrap this new function. In order to
create the pg_proc entry I need the return type oid. If I understand
correctly, in order to get a composite return type, with a known oid, I
would need to create a bootstrapped relation and the corresponding
bootstrapped pg_type entry.

Is there any alternative? It seems ugly to bootstrap so many objects for
every (future) builtin function which returns a composite type.

Thanks,

Joe

Tom Lane wrote:

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.

I have a patch ready now which I think meets the design requirements
above for the most part. The API is in two pieces: one which aids in
creation of functions which return composite types; the other helps with
SRFs. The comments in funcapi.h summarize the API:

/*-------------------------------------------------------------------------
* Support to ease writing Functions returning composite types
*
* External declarations:
* TupleDesc RelationNameGetTupleDesc(char *relname) - Use to get a
* TupleDesc based on the function's return type relation.
* TupleDesc TypeGetTupleDesc(Oid typeoid, List *colaliases) - Use to
* get a TupleDesc based on the function's type oid. This can be
* used to get a TupleDesc for a base (scalar), or composite type.
* TupleTableSlot *TupleDescGetSlot(TupleDesc tupdesc) - Initialize a
* slot given a TupleDesc.
* AttInMetadata *TupleDescGetAttInMetadata(TupleDesc tupdesc) - Get a
* pointer to AttInMetadata based on the function's TupleDesc.
* AttInMetadata can be used in conjunction with C strings to
* produce a properly formed tuple. Store the metadata here for
* use across calls to avoid redundant work.
* HeapTuple BuildTupleFromCStrings(AttInMetadata *attinmeta,
* char **values) -
* 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.
*
* Macro declarations:
* TupleGetDatum(TupleTableSlot *slot, HeapTuple tuple) - get a Datum
* given a tuple and a slot.
*/

/*-------------------------------------------------------------------------
* Support for Set Returning Functions (SRFs)
*
* The basic API for SRFs looks something like:
*
* Datum
* my_Set_Returning_Function(PG_FUNCTION_ARGS)
* {
* FuncCallContext *funcctx;
* Datum result;
* <user defined declarations>
*
* if(SRF_IS_FIRSTPASS())
* {
* <user defined code>
* <obtain slot>
* funcctx = SRF_FIRSTCALL_INIT(slot);
* <user defined code>
* }
* <user defined code>
* funcctx = SRF_PERCALL_SETUP(funcctx);
* <user defined code>
*
* if (funcctx->call_cntr < funcctx->max_calls)
* {
* <user defined code>
* <obtain result Datum>
* SRF_RETURN_NEXT(funcctx, result);
* }
* else
* {
* SRF_RETURN_DONE(funcctx);
* }
* }
*
*/

If interested in more details, the patch will be sent shortly to
PATCHES. Included in the patch is a reference implementation of the API,
show_all_vars() (showguc_all() in guc.c). This returns the same
information as SHOW ALL, but as an SRF, allowing, for example, the
following:

test=# select * from show_all_vars() where varname like 'cpu%';
varname | varval
----------------------+--------
cpu_index_tuple_cost | 0.001
cpu_operator_cost | 0.0025
cpu_tuple_cost | 0.01
(3 rows)

Comments/thoughts?

Thanks,

Joe

Tom Lane wrote:

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.

As promised on HACKERS, here's the Composite and SRF function API patch.
Included is a new builtin guc function, exported as show_all_vars(). In
order to avoid creating a new bootstrapped relation, I made the pg_proc
entry specify 0 as the function return type, and then fixed it in
initdb.sh as follows:

$ECHO_N "setting return type for composite returning functions... "$ECHO_C

"$PGPATH"/postgres $PGSQL_OPT template1 >/dev/null <<EOF
CREATE VIEW pg__guc AS \
SELECT \
''::text as varname, \
''::text as varval;

UPDATE pg_proc SET \
prorettype = (SELECT oid FROM pg_type WHERE typname = 'pg__guc') \
WHERE \
proname = 'show_all_vars';

EOF
if [ "$?" -ne 0 ]; then
exit_nicely
fi
echo "ok"

Any concerns with this approach? Is it too much of a hack, or
preferrable to adding new bootstrapped relations solely for this purpose?

Thanks,

Joe

Joe Conway <mail@joeconway.com> writes:

The question is how to best bootstrap this new function. In order to
create the pg_proc entry I need the return type oid. If I understand
correctly, in order to get a composite return type, with a known oid, I
would need to create a bootstrapped relation and the corresponding
bootstrapped pg_type entry.

Well, we're not doing that; and I see no good reason to make the thing
be a builtin function at all. Since it's just an example, it can very
well be a contrib item with a creation script. Probably *should* be,
in fact, because dynamically created functions are what other people are
going to be building; an example of how to do it as a builtin function
isn't as helpful.

Further down the road it may be that we'll get around to allowing
freestanding composite types (ie, ones with no associated table).
That would make it less painful to have builtin functions returning
tuples --- though not by a lot, since you'd still have to manufacture
pg_type and pg_attribute rows for 'em by hand. I'm not in a hurry to do
that in any case, because of the extent of restructuring of pg_class,
pg_type, and pg_attribute that would be needed.

regards, tom lane

Tom Lane wrote:

Well, we're not doing that; and I see no good reason to make the thing
be a builtin function at all. Since it's just an example, it can very
well be a contrib item with a creation script. Probably *should* be,
in fact, because dynamically created functions are what other people are
going to be building; an example of how to do it as a builtin function
isn't as helpful.

True enough, although I could always create another example for contrib.
Returning GUC variable "SHOW ALL" results as a query result has been
discussed before, and I thought there was agreement that it was a
desirable backend feature.

Is the approach in my patch still too ugly to allow a builtin SRF (set
the function return type to 0 in pg_proc.h, create a view and fix the
pg_proc entry during initdb)? If so, I'll rework the patch into two
patches: one for the composite/set returning function api, and one for
show_all_vars() as a contrib/SRF example. If not, I'll just come up with
another function for contrib to serve as a reference implementation for
others.

Thanks,

Joe

Joe Conway <mail@joeconway.com> writes:

Returning GUC variable "SHOW ALL" results as a query result has been
discussed before, and I thought there was agreement that it was a
desirable backend feature.

So it is, but I had expected it to be implemented by changing the
behavior of SHOW, same as we did for EXPLAIN.

Is the approach in my patch still too ugly to allow a builtin SRF (set
the function return type to 0 in pg_proc.h, create a view and fix the
pg_proc entry during initdb)?

Too ugly for my taste anyway ...

regards, tom lane

Tom Lane wrote:

Joe Conway <mail@joeconway.com> writes:

Is the approach in my patch still too ugly to allow a builtin SRF (set
the function return type to 0 in pg_proc.h, create a view and fix the
pg_proc entry during initdb)?

Too ugly for my taste anyway ...

OK.

Here is a patch for Composite and Set returning function support. I made
two small changes to the API since last patch, which hopefully completes
the decoupling of composite function support from SRF specific support.
If there are no (further ;-)) objections, please apply. I'll send
another post with a patch for contrib/showguc.

Thanks,

Joe

Tom Lane wrote:

Well, we're not doing that; and I see no good reason to make the thing
be a builtin function at all. Since it's just an example, it can very
well be a contrib item with a creation script. Probably *should* be,
in fact, because dynamically created functions are what other people are
going to be building; an example of how to do it as a builtin function
isn't as helpful.

Here is a patch for contrib/showguc. It can serve as a reference
implementation for a C function which returns setof composite. It
required some small changes in guc.c and guc.h so that the number of GUC
variables, and their values, could be accessed. Example usage as shown
below:

test=# select * from show_all_vars() where varname = 'wal_sync_method';
varname | varval
-----------------+-----------
wal_sync_method | fdatasync
(1 row)

test=# select show_var('wal_sync_method');
show_var
-----------
fdatasync
(1 row)

show_var() is neither composite nor set returning, but it seemed like a
worthwhile addition. Please apply if there are no objections.

Thanks,

Joe

Tom Lane wrote:

Well, we're not doing that; and I see no good reason to make the thing
be a builtin function at all. Since it's just an example, it can very
well be a contrib item with a creation script. Probably *should* be,
in fact, because dynamically created functions are what other people are
going to be building; an example of how to do it as a builtin function
isn't as helpful.

True enough, although I could always create another example for contrib.
Returning GUC variable "SHOW ALL" results as a query result has been
discussed before, and I thought there was agreement that it was a
desirable backend feature.

Sure would be. Means we can show config variables nicely in phpPgAdmin like
phpMyAdmin does...

Chris