Re: Table Function API patch (was Re: another SRF question)

Patch applied. Thanks.

---------------------------------------------------------------------------

Joe Conway wrote:
> Neil Conway wrote:
> > On Mon, Jul 08, 2002 at 03:52:24PM -0700, Joe Conway wrote:
> >>Nice catch. Here's (I think) a proper fix. This assumes that if
> >>(values[i] != NULL), the datum is the output of the appropriate "in"
> >>function, otherwise the datum is NULL. It saves the extra pass through
> >>the dvalues array too.
> >>
> >>I thought you might want to try it out now without waiting for the patch
> >>to get into cvs. I tested it with a contrived test function and it
> >>worked as desired.
> >
> >
> > Great, seems to work fine here. Thanks for fixing that for me.
> >
>
> Here is a patch for the Table Function API. It fixes a bug found by Neil
> Conway (BuildTupleFromCStrings sets NULL for pass-by-value types when
> intended value is 0). It also implements some other improvements
> suggested by Neil.
>
> If there are no objections, please apply.
>
> Thanks,
>
> Joe
>
>

> Index: src/backend/executor/execTuples.c
> ===================================================================
> RCS file: /opt/src/cvs/pgsql/src/backend/executor/execTuples.c,v
> retrieving revision 1.53
> diff -c -r1.53 execTuples.c
> *** src/backend/executor/execTuples.c 20 Jun 2002 20:29:27 -0000 1.53
> --- src/backend/executor/execTuples.c 8 Jul 2002 22:25:14 -0000
> ***************
> *** 759,764 ****
> --- 759,765 ----
> natts = tupdesc->natts;
>
> dvalues = (Datum *) palloc(natts * sizeof(Datum));
> + nulls = (char *) palloc(natts * sizeof(char));
>
> /* Call the "in" function for each attribute */
> for (i = 0; i < natts; i++)
> ***************
> *** 772,793 ****
> dvalues[i] = FunctionCall3(&attinfuncinfo, CStringGetDatum(values[i]),
> ObjectIdGetDatum(attelem),
> Int32GetDatum(atttypmod));
> }
> else
> dvalues[i] = PointerGetDatum(NULL);
> }
>
> /*
> * Form a tuple
> */
> - nulls = (char *) palloc(natts * sizeof(char));
> - for (i = 0; i < natts; i++)
> - {
> - if (DatumGetPointer(dvalues[i]) != NULL)
> - nulls[i] = ' ';
> - else
> - nulls[i] = 'n';
> - }
> tuple = heap_formtuple(tupdesc, dvalues, nulls);
>
> return tuple;
> --- 773,790 ----
> dvalues[i] = FunctionCall3(&attinfuncinfo, CStringGetDatum(values[i]),
> ObjectIdGetDatum(attelem),
> Int32GetDatum(atttypmod));
> + nulls[i] = ' ';
> }
> else
> + {
> dvalues[i] = PointerGetDatum(NULL);
> + nulls[i] = 'n';
> + }
> }
>
> /*
> * Form a tuple
> */
> tuple = heap_formtuple(tupdesc, dvalues, nulls);
>
> return tuple;
> Index: src/backend/utils/fmgr/funcapi.c
> ===================================================================
> RCS file: /opt/src/cvs/pgsql/src/backend/utils/fmgr/funcapi.c,v
> retrieving revision 1.1
> diff -c -r1.1 funcapi.c
> *** src/backend/utils/fmgr/funcapi.c 20 Jun 2002 20:37:00 -0000 1.1
> --- src/backend/utils/fmgr/funcapi.c 9 Jul 2002 18:21:15 -0000
> ***************
> *** 52,58 ****
> retval->call_cntr = 0;
> retval->max_calls = 0;
> retval->slot = NULL;
> ! retval->fctx = NULL;
> retval->attinmeta = NULL;
> retval->fmctx = fcinfo->flinfo->fn_mcxt;
>
> --- 52,58 ----
> retval->call_cntr = 0;
> retval->max_calls = 0;
> retval->slot = NULL;
> ! retval->user_fctx = NULL;
> retval->attinmeta = NULL;
> retval->fmctx = fcinfo->flinfo->fn_mcxt;
>
> ***************
> *** 71,76 ****
> --- 71,93 ----
> /* never reached, but keep compiler happy */
> retval = NULL;
> }
> +
> + return retval;
> + }
> +
> + /*
> + * per_MultiFuncCall
> + *
> + * Do Multi-function per-call setup
> + */
> + FuncCallContext *
> + per_MultiFuncCall(PG_FUNCTION_ARGS)
> + {
> + FuncCallContext *retval = (FuncCallContext *) fcinfo->flinfo->fn_extra;
> +
> + /* make sure we start with a fresh slot */
> + if(retval->slot != NULL)
> + ExecClearTuple(retval->slot);
>
> return retval;
> }
> Index: src/include/funcapi.h
> ===================================================================
> RCS file: /opt/src/cvs/pgsql/src/include/funcapi.h,v
> retrieving revision 1.2
> diff -c -r1.2 funcapi.h
> *** src/include/funcapi.h 22 Jun 2002 04:08:07 -0000 1.2
> --- src/include/funcapi.h 9 Jul 2002 18:17:27 -0000
> ***************
> *** 65,86 ****
> */
> typedef struct
> {
> ! /* Number of times we've been called before */
> uint32 call_cntr;
>
> ! /* Maximum number of calls */
> uint32 max_calls;
>
> ! /* pointer to result slot */
> TupleTableSlot *slot;
>
> ! /* pointer to misc context info */
> ! void *fctx;
> !
> ! /* pointer to struct containing arrays of attribute type input metainfo */
> AttInMetadata *attinmeta;
>
> ! /* memory context used to initialize structure */
> MemoryContext fmctx;
>
> } FuncCallContext;
> --- 65,121 ----
> */
> typedef struct
> {
> ! /*
> ! * Number of times we've been called before.
> ! *
> ! * call_cntr is initialized to 0 for you by SRF_FIRSTCALL_INIT(), and
> ! * incremented for you every time SRF_RETURN_NEXT() is called.
> ! */
> uint32 call_cntr;
>
> ! /*
> ! * OPTIONAL maximum number of calls
> ! *
> ! * max_calls is here for convenience ONLY and setting it is OPTIONAL.
> ! * If not set, you must provide alternative means to know when the
> ! * function is done.
> ! */
> uint32 max_calls;
>
> ! /*
> ! * OPTIONAL pointer to result slot
> ! *
> ! * slot is for use when returning tuples (i.e. composite data types)
> ! * and is not needed when returning base (i.e. scalar) data types.
> ! */
> TupleTableSlot *slot;
>
> ! /*
> ! * OPTIONAL pointer to misc user provided context info
> ! *
> ! * user_fctx is for use as a pointer to your own struct to retain
> ! * arbitrary context information between calls for your function.
> ! */
> ! void *user_fctx;
> !
> ! /*
> ! * OPTIONAL pointer to struct containing arrays of attribute type input
> ! * metainfo
> ! *
> ! * attinmeta is for use when returning tuples (i.e. composite data types)
> ! * and is not needed when returning base (i.e. scalar) data types. It
> ! * is ONLY needed if you intend to use BuildTupleFromCStrings() to create
> ! * the return tuple.
> ! */
> AttInMetadata *attinmeta;
>
> ! /*
> ! * memory context used to initialize structure
> ! *
> ! * fmctx is set by SRF_FIRSTCALL_INIT() for you, and used by
> ! * SRF_RETURN_DONE() for cleanup. It is primarily for internal use
> ! * by the API.
> ! */
> MemoryContext fmctx;
>
> } FuncCallContext;
> ***************
> *** 137,143 ****
> * Datum result;
> * <user defined declarations>
> *
> ! * if(SRF_IS_FIRSTPASS())
> * {
> * <user defined code>
> * funcctx = SRF_FIRSTCALL_INIT();
> --- 172,178 ----
> * Datum result;
> * <user defined declarations>
> *
> ! * if(SRF_IS_FIRSTCALL())
> * {
> * <user defined code>
> * funcctx = SRF_FIRSTCALL_INIT();
> ***************
> *** 148,154 ****
> * <user defined code>
> * }
> * <user defined code>
> ! * funcctx = SRF_PERCALL_SETUP(funcctx);
> * <user defined code>
> *
> * if (funcctx->call_cntr < funcctx->max_calls)
> --- 183,189 ----
> * <user defined code>
> * }
> * <user defined code>
> ! * funcctx = SRF_PERCALL_SETUP();
> * <user defined code>
> *
> * if (funcctx->call_cntr < funcctx->max_calls)
> ***************
> *** 167,180 ****
>
> /* from funcapi.c */
> extern FuncCallContext *init_MultiFuncCall(PG_FUNCTION_ARGS);
> extern void end_MultiFuncCall(PG_FUNCTION_ARGS, FuncCallContext *funcctx);
>
> ! #define SRF_IS_FIRSTPASS() (fcinfo->flinfo->fn_extra == NULL)
> #define SRF_FIRSTCALL_INIT() init_MultiFuncCall(fcinfo)
> ! #define SRF_PERCALL_SETUP(_funcctx) \
> ! fcinfo->flinfo->fn_extra; \
> ! if(_funcctx->slot != NULL) \
> ! ExecClearTuple(_funcctx->slot)
> #define SRF_RETURN_NEXT(_funcctx, _result) \
> do { \
> ReturnSetInfo *rsi; \
> --- 202,213 ----
>
> /* from funcapi.c */
> extern FuncCallContext *init_MultiFuncCall(PG_FUNCTION_ARGS);
> + extern FuncCallContext *per_MultiFuncCall(PG_FUNCTION_ARGS);
> extern void end_MultiFuncCall(PG_FUNCTION_ARGS, FuncCallContext *funcctx);
>
> ! #define SRF_IS_FIRSTCALL() (fcinfo->flinfo->fn_extra == NULL)
> #define SRF_FIRSTCALL_INIT() init_MultiFuncCall(fcinfo)
> ! #define SRF_PERCALL_SETUP() per_MultiFuncCall(fcinfo)
> #define SRF_RETURN_NEXT(_funcctx, _result) \
> do { \
> ReturnSetInfo *rsi; \

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026