Thread: SRF memory mgmt patch (was [HACKERS] Concern about memory management with SRFs)
SRF memory mgmt patch (was [HACKERS] Concern about memory management with SRFs)
From
Joe Conway
Date:
Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>>Is it OK to use fcinfo->flinfo->fn_mcxt as the long term memory
>>context or is there a better choice?
>
> That is the correct choice.
>
>>Is funcctx->multi_call_memory_ctx a
>>suitable name in place of funcctx->fmctx?
>
> No objection here.
Here's a patch to address Tom's SRF API memory management concerns, as
discussed earlier today on HACKERS.
Please note that, although this should apply cleanly on cvs tip, it will
have (two) failed hunks (nodeFunctionscan.c) *if* applied after Neil's
plpgsql SRF patch. Or it will cause a failure in Neil's patch if it is
applied first (I think). The fix in either case is to wrap the loop that
collects rows from the function and stashes them in the tuplestore as
follows:
do until no more tuples
+ ExprContext *econtext = scanstate->csstate.cstate.cs_ExprContext;
get one tuple
put it in the tuplestore
+ ResetExprContext(econtext);
loop
Also note that contrib/dblink is intentionally missing, because I'm
still working on other aspects of it. I'll have an updated dblink in a
day or so.
If there are no objections, please apply.
Thanks,
Joe
Index: contrib/pgstattuple/pgstattuple.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/contrib/pgstattuple/pgstattuple.c,v
retrieving revision 1.7
diff -c -r1.7 pgstattuple.c
*** contrib/pgstattuple/pgstattuple.c 23 Aug 2002 08:19:49 -0000 1.7
--- contrib/pgstattuple/pgstattuple.c 29 Aug 2002 05:31:26 -0000
***************
*** 78,94 ****
TupleDesc tupdesc;
TupleTableSlot *slot;
AttInMetadata *attinmeta;
!
! char **values;
! int i;
! Datum result;
/* stuff done only on the first call of the function */
if(SRF_IS_FIRSTCALL())
{
/* create a function context for cross-call persistence */
! funcctx = SRF_FIRSTCALL_INIT();
!
/* total number of tuples to be returned */
funcctx->max_calls = 1;
--- 78,97 ----
TupleDesc tupdesc;
TupleTableSlot *slot;
AttInMetadata *attinmeta;
! char **values;
! int i;
! Datum result;
! MemoryContext oldcontext;
/* stuff done only on the first call of the function */
if(SRF_IS_FIRSTCALL())
{
/* create a function context for cross-call persistence */
! funcctx = SRF_FIRSTCALL_INIT();
!
! /* switch to memory context appropriate for multiple function calls */
! oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
!
/* total number of tuples to be returned */
funcctx->max_calls = 1;
***************
*** 109,114 ****
--- 112,119 ----
*/
attinmeta = TupleDescGetAttInMetadata(tupdesc);
funcctx->attinmeta = attinmeta;
+
+ MemoryContextSwitchTo(oldcontext);
}
/* stuff done on every call of the function */
Index: contrib/tablefunc/tablefunc.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/contrib/tablefunc/tablefunc.c,v
retrieving revision 1.2
diff -c -r1.2 tablefunc.c
*** contrib/tablefunc/tablefunc.c 15 Aug 2002 02:51:26 -0000 1.2
--- contrib/tablefunc/tablefunc.c 29 Aug 2002 05:37:30 -0000
***************
*** 87,92 ****
--- 87,93 ----
float8 stddev;
float8 carry_val;
bool use_carry;
+ MemoryContext oldcontext;
/* stuff done only on the first call of the function */
if(SRF_IS_FIRSTCALL())
***************
*** 94,99 ****
--- 95,103 ----
/* create a function context for cross-call persistence */
funcctx = SRF_FIRSTCALL_INIT();
+ /* switch to memory context appropriate for multiple function calls */
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
/* total number of tuples to be returned */
funcctx->max_calls = PG_GETARG_UINT32(0);
***************
*** 119,124 ****
--- 123,130 ----
* purpose it doesn't matter, just cast it as an unsigned value
*/
srandom(PG_GETARG_UINT32(3));
+
+ MemoryContextSwitchTo(oldcontext);
}
/* stuff done on every call of the function */
***************
*** 260,269 ****
AttInMetadata *attinmeta;
SPITupleTable *spi_tuptable = NULL;
TupleDesc spi_tupdesc;
! char *lastrowid;
crosstab_fctx *fctx;
int i;
int num_categories;
/* stuff done only on the first call of the function */
if(SRF_IS_FIRSTCALL())
--- 266,276 ----
AttInMetadata *attinmeta;
SPITupleTable *spi_tuptable = NULL;
TupleDesc spi_tupdesc;
! char *lastrowid = NULL;
crosstab_fctx *fctx;
int i;
int num_categories;
+ MemoryContext oldcontext;
/* stuff done only on the first call of the function */
if(SRF_IS_FIRSTCALL())
***************
*** 275,287 ****
TupleDesc tupdesc = NULL;
int ret;
int proc;
- MemoryContext oldcontext;
/* create a function context for cross-call persistence */
funcctx = SRF_FIRSTCALL_INIT();
! /* SPI switches context on us, so save it first */
! oldcontext = CurrentMemoryContext;
/* Connect to SPI manager */
if ((ret = SPI_connect()) < 0)
--- 282,293 ----
TupleDesc tupdesc = NULL;
int ret;
int proc;
/* create a function context for cross-call persistence */
funcctx = SRF_FIRSTCALL_INIT();
! /* switch to memory context appropriate for multiple function calls */
! oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
/* Connect to SPI manager */
if ((ret = SPI_connect()) < 0)
***************
*** 317,324 ****
SRF_RETURN_DONE(funcctx);
}
! /* back to the original memory context */
! MemoryContextSwitchTo(oldcontext);
/* get the typeid that represents our return type */
functypeid = get_func_rettype(funcid);
--- 323,330 ----
SRF_RETURN_DONE(funcctx);
}
! /* SPI switches context on us, so reset it */
! MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
/* get the typeid that represents our return type */
functypeid = get_func_rettype(funcid);
***************
*** 381,386 ****
--- 387,394 ----
/* total number of tuples to be returned */
funcctx->max_calls = proc;
+
+ MemoryContextSwitchTo(oldcontext);
}
/* stuff done on every call of the function */
***************
*** 432,438 ****
for (i = 0; i < num_categories; i++)
{
HeapTuple spi_tuple;
! char *rowid;
/* see if we've gone too far already */
if (call_cntr >= max_calls)
--- 440,446 ----
for (i = 0; i < num_categories; i++)
{
HeapTuple spi_tuple;
! char *rowid = NULL;
/* see if we've gone too far already */
if (call_cntr >= max_calls)
***************
*** 496,502 ****
--- 504,516 ----
xpfree(fctx->lastrowid);
if (values[0] != NULL)
+ {
+ /* switch to memory context appropriate for multiple function calls */
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
lastrowid = fctx->lastrowid = pstrdup(values[0]);
+ MemoryContextSwitchTo(oldcontext);
+ }
if (!allnulls)
{
Index: doc/src/sgml/xfunc.sgml
===================================================================
RCS file: /opt/src/cvs/pgsql-server/doc/src/sgml/xfunc.sgml,v
retrieving revision 1.57
diff -c -r1.57 xfunc.sgml
*** doc/src/sgml/xfunc.sgml 29 Aug 2002 00:17:02 -0000 1.57
--- doc/src/sgml/xfunc.sgml 29 Aug 2002 05:30:04 -0000
***************
*** 1670,1682 ****
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;
</programlisting>
--- 1670,1682 ----
AttInMetadata *attinmeta;
/*
! * memory context used for structures which must live for multiple calls
*
! * multi_call_memory_ctx is set by SRF_FIRSTCALL_INIT() for you, and used
! * by SRF_RETURN_DONE() for cleanup. It is the most appropriate memory
! * context for any memory that is allocated for use in multiple calls.
*/
! MemoryContext multi_call_memory_ctx;
} FuncCallContext;
</programlisting>
***************
*** 1720,1740 ****
Datum
my_Set_Returning_Function(PG_FUNCTION_ARGS)
{
! FuncCallContext *funcctx;
! Datum result;
[user defined declarations]
if (SRF_IS_FIRSTCALL())
{
/* one-time setup code appears here: */
[user defined code]
- funcctx = SRF_FIRSTCALL_INIT();
[if returning composite]
[build TupleDesc, and perhaps AttInMetadata]
[obtain slot]
funcctx->slot = slot;
[endif returning composite]
[user defined code]
}
/* each-time setup code appears here: */
--- 1720,1743 ----
Datum
my_Set_Returning_Function(PG_FUNCTION_ARGS)
{
! FuncCallContext *funcctx;
! Datum result;
! MemoryContext oldcontext;
[user defined declarations]
if (SRF_IS_FIRSTCALL())
{
+ funcctx = SRF_FIRSTCALL_INIT();
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
/* one-time setup code appears here: */
[user defined code]
[if returning composite]
[build TupleDesc, and perhaps AttInMetadata]
[obtain slot]
funcctx->slot = slot;
[endif returning composite]
[user defined code]
+ MemoryContextSwitchTo(oldcontext);
}
/* each-time setup code appears here: */
***************
*** 1747,1752 ****
--- 1750,1758 ----
{
/* here we want to return another item: */
[user defined code]
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+ [user allocated memory needed in later function calls]
+ MemoryContextSwitchTo(oldcontext);
[obtain result Datum]
SRF_RETURN_NEXT(funcctx, result);
}
***************
*** 1777,1784 ****
/* stuff done only on the first call of the function */
if (SRF_IS_FIRSTCALL())
{
! /* create a function context for cross-call persistence */
! funcctx = SRF_FIRSTCALL_INIT();
/* total number of tuples to be returned */
funcctx->max_calls = PG_GETARG_UINT32(0);
--- 1783,1795 ----
/* stuff done only on the first call of the function */
if (SRF_IS_FIRSTCALL())
{
! MemoryContext oldcontext;
!
! /* create a function context for cross-call persistence */
! funcctx = SRF_FIRSTCALL_INIT();
!
! /* switch to memory context appropriate for multiple function calls */
! oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
/* total number of tuples to be returned */
funcctx->max_calls = PG_GETARG_UINT32(0);
***************
*** 1800,1805 ****
--- 1811,1818 ----
*/
attinmeta = TupleDescGetAttInMetadata(tupdesc);
funcctx->attinmeta = attinmeta;
+
+ MemoryContextSwitchTo(oldcontext);
}
/* stuff done on every call of the function */
Index: src/backend/executor/nodeFunctionscan.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/executor/nodeFunctionscan.c,v
retrieving revision 1.6
diff -c -r1.6 nodeFunctionscan.c
*** src/backend/executor/nodeFunctionscan.c 29 Aug 2002 00:17:04 -0000 1.6
--- src/backend/executor/nodeFunctionscan.c 29 Aug 2002 04:38:01 -0000
***************
*** 96,101 ****
--- 96,102 ----
{
bool isNull;
ExprDoneCond isDone;
+ ExprContext *econtext = scanstate->csstate.cstate.cs_ExprContext;
isNull = false;
isDone = ExprSingleResult;
***************
*** 108,113 ****
--- 109,116 ----
if (isDone != ExprMultipleResult)
break;
+
+ ResetExprContext(econtext);
}
/*
Index: src/backend/utils/adt/lockfuncs.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/utils/adt/lockfuncs.c,v
retrieving revision 1.3
diff -c -r1.3 lockfuncs.c
*** src/backend/utils/adt/lockfuncs.c 29 Aug 2002 00:17:05 -0000 1.3
--- src/backend/utils/adt/lockfuncs.c 29 Aug 2002 05:37:32 -0000
***************
*** 24,38 ****
Datum
pg_lock_status(PG_FUNCTION_ARGS)
{
! FuncCallContext *funccxt;
! LockData *lockData;
if (SRF_IS_FIRSTCALL())
{
- MemoryContext oldcxt;
TupleDesc tupdesc;
! funccxt = SRF_FIRSTCALL_INIT();
tupdesc = CreateTemplateTupleDesc(5, WITHOUTOID);
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relation",
OIDOID, -1, 0, false);
--- 24,43 ----
Datum
pg_lock_status(PG_FUNCTION_ARGS)
{
! FuncCallContext *funcctx;
! LockData *lockData;
! MemoryContext oldcontext;
if (SRF_IS_FIRSTCALL())
{
TupleDesc tupdesc;
! /* create a function context for cross-call persistence */
! funcctx = SRF_FIRSTCALL_INIT();
!
! /* switch to memory context appropriate for multiple function calls */
! oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
!
tupdesc = CreateTemplateTupleDesc(5, WITHOUTOID);
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "relation",
OIDOID, -1, 0, false);
***************
*** 45,54 ****
TupleDescInitEntry(tupdesc, (AttrNumber) 5, "isgranted",
BOOLOID, -1, 0, false);
! funccxt->slot = TupleDescGetSlot(tupdesc);
! funccxt->attinmeta = TupleDescGetAttInMetadata(tupdesc);
!
! oldcxt = MemoryContextSwitchTo(funccxt->fmctx);
/*
* Preload all the locking information that we will eventually format
--- 50,57 ----
TupleDescInitEntry(tupdesc, (AttrNumber) 5, "isgranted",
BOOLOID, -1, 0, false);
! funcctx->slot = TupleDescGetSlot(tupdesc);
! funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
/*
* Preload all the locking information that we will eventually format
***************
*** 56,70 ****
* MemoryContext is reset when the SRF finishes, we don't need to
* free it ourselves.
*/
! funccxt->user_fctx = (LockData *) palloc(sizeof(LockData));
! GetLockStatusData(funccxt->user_fctx);
! MemoryContextSwitchTo(oldcxt);
}
! funccxt = SRF_PERCALL_SETUP();
! lockData = (LockData *) funccxt->user_fctx;
while (lockData->currIdx < lockData->nelements)
{
--- 59,73 ----
* MemoryContext is reset when the SRF finishes, we don't need to
* free it ourselves.
*/
! funcctx->user_fctx = (LockData *) palloc(sizeof(LockData));
! GetLockStatusData(funcctx->user_fctx);
! MemoryContextSwitchTo(oldcontext);
}
! funcctx = SRF_PERCALL_SETUP();
! lockData = (LockData *) funcctx->user_fctx;
while (lockData->currIdx < lockData->nelements)
{
***************
*** 82,88 ****
holder = &(lockData->holders[currIdx]);
lock = &(lockData->locks[currIdx]);
proc = &(lockData->procs[currIdx]);
! num_attrs = funccxt->attinmeta->tupdesc->natts;
values = (char **) palloc(sizeof(*values) * num_attrs);
--- 85,91 ----
holder = &(lockData->holders[currIdx]);
lock = &(lockData->locks[currIdx]);
proc = &(lockData->procs[currIdx]);
! num_attrs = funcctx->attinmeta->tupdesc->natts;
values = (char **) palloc(sizeof(*values) * num_attrs);
***************
*** 133,144 ****
strncpy(values[3], GetLockmodeName(mode), 32);
! tuple = BuildTupleFromCStrings(funccxt->attinmeta, values);
! result = TupleGetDatum(funccxt->slot, tuple);
! SRF_RETURN_NEXT(funccxt, result);
}
! SRF_RETURN_DONE(funccxt);
}
static LOCKMODE
--- 136,147 ----
strncpy(values[3], GetLockmodeName(mode), 32);
! tuple = BuildTupleFromCStrings(funcctx->attinmeta, values);
! result = TupleGetDatum(funcctx->slot, tuple);
! SRF_RETURN_NEXT(funcctx, result);
}
! SRF_RETURN_DONE(funcctx);
}
static LOCKMODE
Index: src/backend/utils/fmgr/funcapi.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/utils/fmgr/funcapi.c,v
retrieving revision 1.3
diff -c -r1.3 funcapi.c
*** src/backend/utils/fmgr/funcapi.c 29 Aug 2002 00:17:05 -0000 1.3
--- src/backend/utils/fmgr/funcapi.c 29 Aug 2002 04:32:33 -0000
***************
*** 59,72 ****
retval->slot = NULL;
retval->user_fctx = NULL;
retval->attinmeta = NULL;
! retval->fmctx = fcinfo->flinfo->fn_mcxt;
/*
* save the pointer for cross-call use
*/
fcinfo->flinfo->fn_extra = retval;
- /* back to the original memory context */
MemoryContextSwitchTo(oldcontext);
}
else /* second and subsequent calls */
--- 59,71 ----
retval->slot = NULL;
retval->user_fctx = NULL;
retval->attinmeta = NULL;
! retval->multi_call_memory_ctx = fcinfo->flinfo->fn_mcxt;
/*
* save the pointer for cross-call use
*/
fcinfo->flinfo->fn_extra = retval;
MemoryContextSwitchTo(oldcontext);
}
else /* second and subsequent calls */
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.85
diff -c -r1.85 guc.c
*** src/backend/utils/misc/guc.c 29 Aug 2002 00:17:05 -0000 1.85
--- src/backend/utils/misc/guc.c 29 Aug 2002 05:37:32 -0000
***************
*** 2421,2426 ****
--- 2421,2427 ----
int max_calls;
TupleTableSlot *slot;
AttInMetadata *attinmeta;
+ MemoryContext oldcontext;
/* stuff done only on the first call of the function */
if(SRF_IS_FIRSTCALL())
***************
*** 2428,2433 ****
--- 2429,2437 ----
/* create a function context for cross-call persistence */
funcctx = SRF_FIRSTCALL_INIT();
+ /* switch to memory context appropriate for multiple function calls */
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
/* need a tuple descriptor representing two TEXT columns */
tupdesc = CreateTemplateTupleDesc(2, WITHOUTOID);
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name",
***************
*** 2450,2455 ****
--- 2454,2461 ----
/* total number of tuples to be returned */
funcctx->max_calls = GetNumConfigOptions();
+
+ MemoryContextSwitchTo(oldcontext);
}
/* stuff done on every call of the function */
Index: src/include/funcapi.h
===================================================================
RCS file: /opt/src/cvs/pgsql-server/src/include/funcapi.h,v
retrieving revision 1.5
diff -c -r1.5 funcapi.h
*** src/include/funcapi.h 29 Aug 2002 00:17:06 -0000 1.5
--- src/include/funcapi.h 29 Aug 2002 05:37:32 -0000
***************
*** 101,113 ****
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;
--- 101,113 ----
AttInMetadata *attinmeta;
/*
! * memory context used for structures which must live for multiple calls
*
! * multi_call_memory_ctx is set by SRF_FIRSTCALL_INIT() for you, and used
! * by SRF_RETURN_DONE() for cleanup. It is the most appropriate memory
! * context for any memory that is allocated for use in multiple calls.
*/
! MemoryContext multi_call_memory_ctx;
} FuncCallContext;
***************
*** 160,176 ****
* {
* FuncCallContext *funcctx;
* Datum result;
* <user defined declarations>
*
* if(SRF_IS_FIRSTCALL())
* {
- * <user defined code>
* funcctx = SRF_FIRSTCALL_INIT();
* <if returning composite>
* <obtain slot>
* funcctx->slot = slot;
* <endif returning composite>
* <user defined code>
* }
* <user defined code>
* funcctx = SRF_PERCALL_SETUP();
--- 160,179 ----
* {
* FuncCallContext *funcctx;
* Datum result;
+ * MemoryContext oldcontext;
* <user defined declarations>
*
* if(SRF_IS_FIRSTCALL())
* {
* funcctx = SRF_FIRSTCALL_INIT();
+ * oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+ * <user defined code>
* <if returning composite>
* <obtain slot>
* funcctx->slot = slot;
* <endif returning composite>
* <user defined code>
+ * MemoryContextSwitchTo(oldcontext);
* }
* <user defined code>
* funcctx = SRF_PERCALL_SETUP();
***************
*** 179,184 ****
--- 182,190 ----
* if (funcctx->call_cntr < funcctx->max_calls)
* {
* <user defined code>
+ * oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+ * <user allocated memory needed in later function calls>
+ * MemoryContextSwitchTo(oldcontext);
* <obtain result Datum>
* SRF_RETURN_NEXT(funcctx, result);
* }
Re: SRF memory mgmt patch (was [HACKERS] Concern about memory management with SRFs)
From
Tom Lane
Date:
Joe Conway <mail@joeconway.com> writes:
> Here's a patch to address Tom's SRF API memory management concerns, as
> discussed earlier today on HACKERS.
Patch committed.
It seemed to me that pgstattuple.c does not really want to be an SRF,
but only a function returning a single tuple. As such, it can provide
a fine example of using the funcapi.h tuple-building machinery *without*
the SRF machinery. I changed it accordingly, but am not able to update
README.pgstattuple.euc_jp; Tatsuo-san, would you handle that?
regards, tom lane
Tom Lane wrote:
> Patch committed.
Given the change to SRF memory management, is the following still
necessary (or possibly even incorrect)?
in funcapi.c:
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;
}
All but one of the SRFs I've tried don't seem to care, but I have one
that is getting an assertion:
0x42029331 in kill () from /lib/i686/libc.so.6
(gdb) bt
#0 0x42029331 in kill () from /lib/i686/libc.so.6
#1 0x4202911a in raise () from /lib/i686/libc.so.6
#2 0x4202a8c2 in abort () from /lib/i686/libc.so.6
#3 0x08179ab9 in ExceptionalCondition () at assert.c:48
#4 0x0818416f in pfree (pointer=0x7f7f7f7f) at mcxt.c:470
#5 0x0806bd32 in heap_freetuple (htup=0x832bb80) at heaptuple.c:736
#6 0x080e47df in ExecClearTuple (slot=0x832b2cc) at execTuples.c:406
#7 0x0817cf49 in per_MultiFuncCall (fcinfo=0xbfffe8e0) at funcapi.c:88
#8 0x40017273 in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:911
Not quite sure why yet, but I'm thinking the ExecClearTuple() is no
longer needed/desired anyway.
Joe
Re: SRF memory mgmt patch (was [HACKERS] Concern about memory management with SRFs)
From
Tom Lane
Date:
Joe Conway <mail@joeconway.com> writes:
> Given the change to SRF memory management, is the following still
> necessary (or possibly even incorrect)?
> /* make sure we start with a fresh slot */
> if(retval->slot != NULL)
> ExecClearTuple(retval->slot);
I don't think it was ever necessary... but there's nothing wrong with
it either.
> All but one of the SRFs I've tried don't seem to care, but I have one
> that is getting an assertion:
> 0x42029331 in kill () from /lib/i686/libc.so.6
> (gdb) bt
> #0 0x42029331 in kill () from /lib/i686/libc.so.6
> #1 0x4202911a in raise () from /lib/i686/libc.so.6
> #2 0x4202a8c2 in abort () from /lib/i686/libc.so.6
> #3 0x08179ab9 in ExceptionalCondition () at assert.c:48
> #4 0x0818416f in pfree (pointer=0x7f7f7f7f) at mcxt.c:470
> #5 0x0806bd32 in heap_freetuple (htup=0x832bb80) at heaptuple.c:736
> #6 0x080e47df in ExecClearTuple (slot=0x832b2cc) at execTuples.c:406
> #7 0x0817cf49 in per_MultiFuncCall (fcinfo=0xbfffe8e0) at funcapi.c:88
> #8 0x40017273 in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:911
> Not quite sure why yet, but I'm thinking the ExecClearTuple() is no
> longer needed/desired anyway.
You'll need to fix that anyway because the next ExecStoreTuple will try
to do an ExecClearTuple. Looks like the same tuple is being freed
twice. Once you've handed a tuple to ExecStoreTuple it's not yours to
free anymore; perhaps some bit of code in dblink has that wrong?
regards, tom lane
Tom Lane wrote: > You'll need to fix that anyway because the next ExecStoreTuple will try > to do an ExecClearTuple. Looks like the same tuple is being freed > twice. Once you've handed a tuple to ExecStoreTuple it's not yours to > free anymore; perhaps some bit of code in dblink has that wrong? That's just it: 0x40017273 in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:911 *is* funcctx = SRF_PERCALL_SETUP(); which is is a macro #define SRF_PERCALL_SETUP() per_MultiFuncCall(fcinfo) When I remove the call to ExecClearTuple() from per_MultiFuncCall(), everything starts to work. As you said, if the next ExecStoreTuple will try to do an ExecClearTuple(), ISTM that it should be removed from per_MultiFuncCall()/SRF_PERCALL_SETUP(). Or am I crazy? Joe
Re: SRF memory mgmt patch (was [HACKERS] Concern about memory management with SRFs)
From
Tom Lane
Date:
Joe Conway <mail@joeconway.com> writes:
> As you said, if the next ExecStoreTuple will try to do an
> ExecClearTuple(), ISTM that it should be removed from
> per_MultiFuncCall()/SRF_PERCALL_SETUP().
No, it's not necessary: ExecClearTuple knows the difference between a
full and an empty TupleSlot.
I'm not sure where the excess free is coming from, but it ain't
ExecClearTuple's fault. You might try setting a breakpoint at
heap_freetuple to see if that helps.
regards, tom lane
Re: SRF memory mgmt patch (was [HACKERS] Concern about memory management with SRFs)
From
Tom Lane
Date:
Joe Conway <mail@joeconway.com> writes:
> As you said, if the next ExecStoreTuple will try to do an
> ExecClearTuple(), ISTM that it should be removed from
> per_MultiFuncCall()/SRF_PERCALL_SETUP(). Or am I crazy?
Actually ... on second thought ...
I bet the real issue here is that we have a long-lived TupleTableSlot
pointing at a short-lived tuple. (I assume you're just forming the
tuple in the function's working context, no?)
When ExecClearTuple is called on the next time through, it tries to
pfree a tuple that has already been recycled along with the rest of
the short-term context. Result: coredump.
However, if that were the story then *none* of the SRFs returning
tuple should work, and they do. So I'm still confused.
But I suspect that what we want to do is take management of the tuples
away from the Slot: pass should_free = FALSE to ExecStoreTuple in the
TupleGetDatum macro. The ClearTuple call *is* appropriate when you do
that, because it will reset the Slot to empty rather than leaving it
containing a dangling pointer to a long-since-freed tuple.
regards, tom lane
Tom Lane wrote: > Joe Conway <mail@joeconway.com> writes: > >>As you said, if the next ExecStoreTuple will try to do an >>ExecClearTuple(), ISTM that it should be removed from >>per_MultiFuncCall()/SRF_PERCALL_SETUP(). Or am I crazy? > > > Actually ... on second thought ... > > I bet the real issue here is that we have a long-lived TupleTableSlot > pointing at a short-lived tuple. (I assume you're just forming the > tuple in the function's working context, no?) yep > When ExecClearTuple is called on the next time through, it tries to > pfree a tuple that has already been recycled along with the rest of > the short-term context. Result: coredump. > > However, if that were the story then *none* of the SRFs returning > tuple should work, and they do. So I'm still confused. That's what had me confused. I have found the smoking gun, however. I had changed this function from returning setof text, to returning setof two_column_named_composite_type. *However*. I had not dropped and recreated the function with the proper declaration. Once I redeclared the function properly, the coredump went away, even with current per_MultiFuncCall() code. The way I found this was by removing ExecClearTuple() from per_MultiFuncCall(). That allowed the function to return without core dumping, but it gave me one column of garbage -- that finally clued me in. > But I suspect that what we want to do is take management of the tuples > away from the Slot: pass should_free = FALSE to ExecStoreTuple in the > TupleGetDatum macro. The ClearTuple call *is* appropriate when you do > that, because it will reset the Slot to empty rather than leaving it > containing a dangling pointer to a long-since-freed tuple. OK. I'll make that change and leave ExecClearTuple() in per_MultiFuncCall(). Sound like a plan? Joe
Re: SRF memory mgmt patch (was [HACKERS] Concern about memory management with SRFs)
From
Tom Lane
Date:
Joe Conway <mail@joeconway.com> writes:
> I have found the smoking gun, however. I had changed this function from
> returning setof text, to returning setof
> two_column_named_composite_type. *However*. I had not dropped and
> recreated the function with the proper declaration. Once I redeclared
> the function properly, the coredump went away, even with current
> per_MultiFuncCall() code.
Ah. I think the changes I just committed would have helped:
nodeFunctionscan.c now runs a tupledesc_mismatch() check regardless of
whether it thinks the function returns RECORD or not.
>> But I suspect that what we want to do is take management of the tuples
>> away from the Slot: pass should_free = FALSE to ExecStoreTuple in the
>> TupleGetDatum macro. The ClearTuple call *is* appropriate when you do
>> that, because it will reset the Slot to empty rather than leaving it
>> containing a dangling pointer to a long-since-freed tuple.
> OK. I'll make that change and leave ExecClearTuple() in
> per_MultiFuncCall(). Sound like a plan?
First let's see if we can figure out why the code is failing to fail
as it stands. The fact that it's not dumping core says there's
something we don't understand yet ...
regards, tom lane
> Joe Conway <mail@joeconway.com> writes: > > Here's a patch to address Tom's SRF API memory management concerns, as > > discussed earlier today on HACKERS. > > Patch committed. > > It seemed to me that pgstattuple.c does not really want to be an SRF, > but only a function returning a single tuple. Thank you for modifying pgstattuple.c. You are right, it does not want to return more than 1 tuple. > As such, it can provide > a fine example of using the funcapi.h tuple-building machinery *without* > the SRF machinery. I changed it accordingly, but am not able to update > README.pgstattuple.euc_jp; Tatsuo-san, would you handle that? Sure. I'll take care of that. -- Tatsuo Ishii
Tatsuo Ishii wrote: >>It seemed to me that pgstattuple.c does not really want to be an SRF, >>but only a function returning a single tuple. > > Thank you for modifying pgstattuple.c. You are right, it does not want > to return more than 1 tuple. > I noticed that too, but it did occur to me that at some point you might want to make the function return a row for every table in a database. Perhaps even another system view (like pg_locks or pg_settings)? Joe
Tom Lane wrote:
> First let's see if we can figure out why the code is failing to fail
> as it stands. The fact that it's not dumping core says there's
> something we don't understand yet ...
I'm not sure if the attached will help figure it out, but at the very
least it was eye-opening for me. I ran a test on
dblink_get_pkey('foobar') that returns 5 rows. I had a breakpoint set in
ExecClearTuple. I found that ExecClearTuple was called a total of 32
times for 5 returned rows!
Relevant to this discussion was that ExecClearTuple was called three
times, with the same slot pointer, for each function call to
dblink_get_pkey. Once in SRF_PERCALL_SETUP (per_MultiFuncCall), once in
TupleGetDatum (ExecStoreTuple), and once in FunctionNext in the loop
that builds the tuplestore.
Unfortunately I have not been able to get back to a point where I see a
coredump :(. But, that did seem to be related to calling the function
with an inappropriate declaration (now it just gives me garbage instead
of dumping core, even though I reverted the per_MultiFuncCall changes I
made earlier). I'll keep messing with this for a while, but I was hoping
the attached info would lead to some more suggestions of where to be
looking.
Thanks,
Joe
test=# select * from dblink_get_pkey('foobar');
position | colname
----------+---------
1 | f1
2 | f2
3 | f3
4 | f4
5 | f5
(5 rows)
- breakpoint set at ExecClearTuple
- ExecClearTuple called a total of 32 times
========================================
15 sets of 3 calls for 5 returned tuples
========================================
set 1:
------------------------------------------------------------------
this one is from: SRF_PERCALL_SETUP() == per_MultiFuncCall()
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x0817cf49 in per_MultiFuncCall (fcinfo=0xbfffe8e0) at funcapi.c:88
#2 0x400172b3 in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:911
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
this one is from: TupleGetDatum() == ExecStoreTuple()
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f3b18, slot=0x82ea07c, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x4001733f in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:941
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
this one is from: explicit call in FunctionNext()
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x080e9ee6 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:107
#2 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#3 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
set 2:
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x0817cf49 in per_MultiFuncCall (fcinfo=0xbfffe8e0) at funcapi.c:88
#2 0x400172b3 in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:911
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f3ac0, slot=0x82ea07c, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x4001733f in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:941
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x080e9ee6 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:107
#2 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#3 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
set 3:
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x0817cf49 in per_MultiFuncCall (fcinfo=0xbfffe8e0) at funcapi.c:88
#2 0x400172b3 in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:911
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f3ac0, slot=0x82ea07c, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x4001733f in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:941
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x080e9ee6 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:107
#2 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#3 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
set 4:
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x0817cf49 in per_MultiFuncCall (fcinfo=0xbfffe8e0) at funcapi.c:88
#2 0x400172b3 in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:911
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f3ac0, slot=0x82ea07c, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x4001733f in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:941
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x080e9ee6 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:107
#2 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#3 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
set 5:
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x0817cf49 in per_MultiFuncCall (fcinfo=0xbfffe8e0) at funcapi.c:88
#2 0x400172b3 in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:911
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f3ac0, slot=0x82ea07c, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x4001733f in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:941
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x080e9ee6 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:107
#2 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#3 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
=============================================================================
one set from SRF_PERCALL_SETUP() during last pass when it decides it is done
=============================================================================
#0 ExecClearTuple (slot=0x82ea07c) at execTuples.c:395
#1 0x0817cf49 in per_MultiFuncCall (fcinfo=0xbfffe8e0) at funcapi.c:88
#2 0x400172b3 in dblink_get_pkey (fcinfo=0xbfffe8e0) at dblink.c:911
#3 0x080e341a in ExecMakeFunctionResult (fcache=0x82e8d1c, arguments=0x82e8330, econtext=0x82e89bc, isNull=0xbfffeac7
"",
=============================================================================
10 sets of 2 calls for 5 returned tuples
=============================================================================
set 1:
------------------------------------------------------------------
this one is from:
return ExecStoreTuple(...
in FunctionNext
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82e8938) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82ea148, slot=0x82e8938, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x080e9f21 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:127
#3 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
this one is from:
return ExecStoreTuple(...
in ExecProject
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82e8924) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f5bf8, slot=0x82e8924, buffer=0, shouldFree=1 '\001') at execTuples.c:359
#2 0x080e42cf in ExecProject (projInfo=0x82e8ca8, isDone=0xbfffeaf8) at execQual.c:2060
#3 0x080e438b in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:134
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
set 2:
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82e8938) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f5ac8, slot=0x82e8938, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x080e9f21 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:127
#3 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
#0 ExecClearTuple (slot=0x82e8924) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f5d9c, slot=0x82e8924, buffer=0, shouldFree=1 '\001') at execTuples.c:359
#2 0x080e42cf in ExecProject (projInfo=0x82e8ca8, isDone=0xbfffeaf8) at execQual.c:2060
#3 0x080e438b in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:134
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
set 3:
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82e8938) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f5b14, slot=0x82e8938, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x080e9f21 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:127
#3 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
#0 ExecClearTuple (slot=0x82e8924) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f5bf8, slot=0x82e8924, buffer=0, shouldFree=1 '\001') at execTuples.c:359
#2 0x080e42cf in ExecProject (projInfo=0x82e8ca8, isDone=0xbfffeaf8) at execQual.c:2060
#3 0x080e438b in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:134
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
set 4:
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82e8938) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f5b60, slot=0x82e8938, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x080e9f21 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:127
#3 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
#0 ExecClearTuple (slot=0x82e8924) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f5d9c, slot=0x82e8924, buffer=0, shouldFree=1 '\001') at execTuples.c:359
#2 0x080e42cf in ExecProject (projInfo=0x82e8ca8, isDone=0xbfffeaf8) at execQual.c:2060
#3 0x080e438b in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:134
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
set 5:
------------------------------------------------------------------
#0 ExecClearTuple (slot=0x82e8938) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f5bac, slot=0x82e8938, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x080e9f21 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:127
#3 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
#0 ExecClearTuple (slot=0x82e8924) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x82f5bf8, slot=0x82e8924, buffer=0, shouldFree=1 '\001') at execTuples.c:359
#2 0x080e42cf in ExecProject (projInfo=0x82e8ca8, isDone=0xbfffeaf8) at execQual.c:2060
#3 0x080e438b in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:134
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
=============================================================================
one more sets of 2 calls - last pass to get an "all done"?
=============================================================================
#0 ExecClearTuple (slot=0x82e8938) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x0, slot=0x82e8938, buffer=0, shouldFree=0 '\0') at execTuples.c:359
#2 0x080e9f21 in FunctionNext (node=0x82e8798) at nodeFunctionscan.c:127
#3 0x080e4341 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:97
#4 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
#0 ExecClearTuple (slot=0x82e8924) at execTuples.c:395
#1 0x080e4667 in ExecStoreTuple (tuple=0x0, slot=0x82e8924, buffer=0, shouldFree=1 '\001') at execTuples.c:359
#2 0x080e4360 in ExecScan (node=0x82e8798, accessMtd=0x80e9e68 <FunctionNext>) at execScan.c:110
#3 0x080e9f3f in ExecFunctionScan (node=0x82e8798) at nodeFunctionscan.c:146
=============================================================================
2 sets - cleanup time in ExecEndFunctionScan
=============================================================================
#0 ExecClearTuple (slot=0x82e8924) at execTuples.c:395
#1 0x080ea1cc in ExecEndFunctionScan (node=0x82e8798) at nodeFunctionscan.c:318
#0 ExecClearTuple (slot=0x82e8938) at execTuples.c:395
#1 0x080ea1d5 in ExecEndFunctionScan (node=0x82e8798) at nodeFunctionscan.c:319
=============================================================================
2 sets - cleanup time in ExecDropTupleTable
=============================================================================
#0 ExecClearTuple (slot=0x82e8924) at execTuples.c:395
#1 0x080e44c9 in ExecDropTupleTable (table=0x82e8908, shouldFree=1) at execTuples.c:204
#0 ExecClearTuple (slot=0x82e8938) at execTuples.c:395
#1 0x080e44c9 in ExecDropTupleTable (table=0x82e8908, shouldFree=1) at execTuples.c:204
Joe Conway wrote: > Unfortunately I have not been able to get back to a point where I see a > coredump :(. But, that did seem to be related to calling the function > with an inappropriate declaration (now it just gives me garbage instead > of dumping core, even though I reverted the per_MultiFuncCall changes I > made earlier). I'll keep messing with this for a while, but I was hoping > the attached info would lead to some more suggestions of where to be > looking. It's back as of cvs tip. This time, it looks like all table functions are failing in the same manner: #4 0x081845fb in pfree (pointer=0x7f7f7f7f) at mcxt.c:470 #5 0x0806bdb2 in heap_freetuple (htup=0x82fc7b8) at heaptuple.c:736 #6 0x080e4cbf in ExecClearTuple (slot=0x82f92f4) at execTuples.c:406 #7 0x0817d3ad in per_MultiFuncCall (fcinfo=0xbfffe9e0) at funcapi.c:88 #8 0x0814af25 in pg_lock_status (fcinfo=0xbfffe9e0) at lockfuncs.c:69 #9 0x080e3990 in ExecMakeTableFunctionResult (funcexpr=0x82e9fa0, #4 0x081845fb in pfree (pointer=0x7f7f7f7f) at mcxt.c:470 #5 0x0806bdb2 in heap_freetuple (htup=0x82f43a4) at heaptuple.c:736 #6 0x080e4cbf in ExecClearTuple (slot=0x82e9f2c) at execTuples.c:406 #7 0x0817d3ad in per_MultiFuncCall (fcinfo=0xbfffe9e0) at funcapi.c:88 #8 0x40016a4b in dblink_record (fcinfo=0xbfffe9e0) at dblink.c:518 #9 0x080e3990 in ExecMakeTableFunctionResult (funcexpr=0x82e8df8, #4 0x081845fb in pfree (pointer=0x7f7f7f7f) at mcxt.c:470 #5 0x0806bdb2 in heap_freetuple (htup=0x83026f8) at heaptuple.c:736 #6 0x080e4cbf in ExecClearTuple (slot=0x82f71cc) at execTuples.c:406 #7 0x0817d3ad in per_MultiFuncCall (fcinfo=0xbfffe9e0) at funcapi.c:88 #8 0x08181635 in show_all_settings (fcinfo=0xbfffe9e0) at guc.c:2469 #9 0x080e3990 in ExecMakeTableFunctionResult (funcexpr=0x82f64a0, Currently all C language table functions are broken :(, but all sql language table functions seem to work -- which is why regression doesn't fail (pointing out the need to add a select * from pg_settings to a regression test somewhere). I'm looking at this now. I suspect the easy fix is to remove ExecClearTuple from per_MultiFuncCall, but I'll try to understand what's going on first. Joe
Joe Conway wrote: > I'm looking at this now. I suspect the easy fix is to remove > ExecClearTuple from per_MultiFuncCall, but I'll try to understand what's > going on first. > On second thought, *all* functions failing is what you expected, right Tom? I just changed TupleGetDatum() as we discussed: #define TupleGetDatum(_slot, _tuple) \ PointerGetDatum(ExecStoreTuple(_tuple, _slot, InvalidBuffer, false)) and now everything works again. Is this the preferred fix and/or is it worth spending more time to dig into this? Joe
Joe Conway wrote: > Joe Conway wrote: > >> I'm looking at this now. I suspect the easy fix is to remove >> ExecClearTuple from per_MultiFuncCall, but I'll try to understand >> what's going on first. >> > > On second thought, *all* functions failing is what you expected, right > Tom? I just changed TupleGetDatum() as we discussed: > > #define TupleGetDatum(_slot, _tuple) \ > PointerGetDatum(ExecStoreTuple(_tuple, _slot, InvalidBuffer, false)) > > and now everything works again. Is this the preferred fix and/or is it > worth spending more time to dig into this? Here is a patch with the above mentioned fix. It also has an addition to rangefuncs.sql and rangefuncs.out to ensure a C language table function gets tested. I did this by adding SELECT * FROM pg_settings WHERE name LIKE 'enable%'; to the test. I think this should produce reasonably stable results, but obviously will require some maintenance if we add/remove a GUC variable matching this criteria. Alternative suggestions welcomed, but if there are no objections, please commit. Thanks, Joe Index: src/include/funcapi.h =================================================================== RCS file: /opt/src/cvs/pgsql-server/src/include/funcapi.h,v retrieving revision 1.6 diff -c -r1.6 funcapi.h *** src/include/funcapi.h 29 Aug 2002 17:14:33 -0000 1.6 --- src/include/funcapi.h 30 Aug 2002 17:20:49 -0000 *************** *** 148,154 **** extern HeapTuple BuildTupleFromCStrings(AttInMetadata *attinmeta, char **values); #define TupleGetDatum(_slot, _tuple) \ ! PointerGetDatum(ExecStoreTuple(_tuple, _slot, InvalidBuffer, true)) /*---------- --- 148,154 ---- extern HeapTuple BuildTupleFromCStrings(AttInMetadata *attinmeta, char **values); #define TupleGetDatum(_slot, _tuple) \ ! PointerGetDatum(ExecStoreTuple(_tuple, _slot, InvalidBuffer, false)) /*---------- Index: src/test/regress/expected/rangefuncs.out =================================================================== RCS file: /opt/src/cvs/pgsql-server/src/test/regress/expected/rangefuncs.out,v retrieving revision 1.4 diff -c -r1.4 rangefuncs.out *** src/test/regress/expected/rangefuncs.out 29 Aug 2002 00:17:06 -0000 1.4 --- src/test/regress/expected/rangefuncs.out 30 Aug 2002 17:29:45 -0000 *************** *** 1,3 **** --- 1,15 ---- + SELECT * FROM pg_settings WHERE name LIKE 'enable%'; + name | setting + ------------------+--------- + enable_hashjoin | on + enable_indexscan | on + enable_mergejoin | on + enable_nestloop | on + enable_seqscan | on + enable_sort | on + enable_tidscan | on + (7 rows) + CREATE TABLE foo2(fooid int, f2 int); INSERT INTO foo2 VALUES(1, 11); INSERT INTO foo2 VALUES(2, 22); Index: src/test/regress/sql/rangefuncs.sql =================================================================== RCS file: /opt/src/cvs/pgsql-server/src/test/regress/sql/rangefuncs.sql,v retrieving revision 1.3 diff -c -r1.3 rangefuncs.sql *** src/test/regress/sql/rangefuncs.sql 29 Aug 2002 00:17:06 -0000 1.3 --- src/test/regress/sql/rangefuncs.sql 30 Aug 2002 17:28:07 -0000 *************** *** 1,3 **** --- 1,5 ---- + SELECT * FROM pg_settings WHERE name LIKE 'enable%'; + CREATE TABLE foo2(fooid int, f2 int); INSERT INTO foo2 VALUES(1, 11); INSERT INTO foo2 VALUES(2, 22);
Joe Conway <mail@joeconway.com> writes:
>> On second thought, *all* functions failing is what you expected, right
>> Tom?
Yeah. I coulda sworn I tested pg_settings yesterday after making those
other changes, but I must not have; it's sure failing for me today.
> Here is a patch with the above mentioned fix. It also has an addition to
> rangefuncs.sql and rangefuncs.out to ensure a C language table function
> gets tested.
Good idea. Will apply.
regards, tom lane
Tom Lane wrote: > Joe Conway <mail@joeconway.com> writes: >>Here is a patch with the above mentioned fix. It also has an addition to >>rangefuncs.sql and rangefuncs.out to ensure a C language table function >>gets tested. > > Good idea. Will apply. BTW, Neil, do you have a sample plpgsql table function that can be included in the rangefuncs regression test? Thanks, Joe
Joe Conway <mail@joeconway.com> writes:
> BTW, Neil, do you have a sample plpgsql table function that can be
> included in the rangefuncs regression test?
The plpgsql regression test has 'em, down at the end.
regards, tom lane