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

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

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

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@postgresql.org

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

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@postgresql.org

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