diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c index 8daac9e4f5..0e8c947e9e 100644 --- a/src/backend/access/common/printtup.c +++ b/src/backend/access/common/printtup.c @@ -21,6 +21,7 @@ #include "tcop/pquery.h" #include "utils/lsyscache.h" #include "utils/memdebug.h" +#include "utils/memutils.h" static void printtup_startup(DestReceiver *self, int operation, @@ -61,6 +62,7 @@ typedef struct TupleDesc attrinfo; /* The attr info we are set up for */ int nattrs; PrinttupAttrInfo *myinfo; /* Cached info about each attr */ + MemoryContext tmpcontext; /* Memory context for per-row workspace */ } DR_printtup; /* ---------------- @@ -87,6 +89,7 @@ printtup_create_DR(CommandDest dest) self->attrinfo = NULL; self->nattrs = 0; self->myinfo = NULL; + self->tmpcontext = NULL; return (DestReceiver *) self; } @@ -124,6 +127,18 @@ printtup_startup(DestReceiver *self, int operation, TupleDesc typeinfo) DR_printtup *myState = (DR_printtup *) self; Portal portal = myState->portal; + /* + * Create a temporary memory context that we can reset once per row to + * recover palloc'd memory. This avoids any problems with leaks inside + * datatype output routines, and should be faster than retail pfree's + * anyway. + */ + myState->tmpcontext = AllocSetContextCreate(CurrentMemoryContext, + "printtup", + ALLOCSET_DEFAULT_MINSIZE, + ALLOCSET_DEFAULT_INITSIZE, + ALLOCSET_DEFAULT_MAXSIZE); + if (PG_PROTOCOL_MAJOR(FrontendProtocol) < 3) { /* @@ -289,6 +304,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self) { TupleDesc typeinfo = slot->tts_tupleDescriptor; DR_printtup *myState = (DR_printtup *) self; + MemoryContext oldcontext; StringInfoData buf; int natts = typeinfo->natts; int i; @@ -300,8 +316,11 @@ printtup(TupleTableSlot *slot, DestReceiver *self) /* Make sure the tuple is fully deconstructed */ slot_getallattrs(slot); + /* Switch into per-row context so we can recover memory below */ + oldcontext = MemoryContextSwitchTo(myState->tmpcontext); + /* - * Prepare a DataRow message + * Prepare a DataRow message (note buffer is in per-row context) */ pq_beginmessage(&buf, 'D'); @@ -313,8 +332,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self) for (i = 0; i < natts; ++i) { PrinttupAttrInfo *thisState = myState->myinfo + i; - Datum origattr = slot->tts_values[i], - attr; + Datum attr = slot->tts_values[i]; if (slot->tts_isnull[i]) { @@ -323,30 +341,15 @@ printtup(TupleTableSlot *slot, DestReceiver *self) } /* - * If we have a toasted datum, forcibly detoast it here to avoid - * memory leakage inside the type's output routine. - * - * Here we catch undefined bytes in tuples that are returned to the + * Here we catch undefined bytes in datums that are returned to the * client without hitting disk; see comments at the related check in - * PageAddItem(). Whether to test before or after detoast is somewhat - * arbitrary, as is whether to test external/compressed data at all. - * Undefined bytes in the pre-toast datum will have triggered Valgrind - * errors in the compressor or toaster; any error detected here for - * such datums would indicate an (unlikely) bug in a type-independent - * facility. Therefore, this test is most useful for uncompressed, - * non-external datums. - * - * We don't presently bother checking non-varlena datums for undefined - * data. PageAddItem() does check them. + * PageAddItem(). This test is most useful for uncompressed, + * non-external datums, but we're quite likely to see such here when + * testing new C functions. */ if (thisState->typisvarlena) - { - VALGRIND_CHECK_MEM_IS_DEFINED(origattr, VARSIZE_ANY(origattr)); - - attr = PointerGetDatum(PG_DETOAST_DATUM(origattr)); - } - else - attr = origattr; + VALGRIND_CHECK_MEM_IS_DEFINED(DatumGetPointer(attr), + VARSIZE_ANY(attr)); if (thisState->format == 0) { @@ -355,7 +358,6 @@ printtup(TupleTableSlot *slot, DestReceiver *self) outputstr = OutputFunctionCall(&thisState->finfo, attr); pq_sendcountedtext(&buf, outputstr, strlen(outputstr), false); - pfree(outputstr); } else { @@ -366,15 +368,14 @@ printtup(TupleTableSlot *slot, DestReceiver *self) pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4); pq_sendbytes(&buf, VARDATA(outputbytes), VARSIZE(outputbytes) - VARHDRSZ); - pfree(outputbytes); } - - /* Clean up detoasted copy, if any */ - if (DatumGetPointer(attr) != DatumGetPointer(origattr)) - pfree(DatumGetPointer(attr)); } pq_endmessage(&buf); + + /* Return to caller's context, and flush row's temporary memory */ + MemoryContextSwitchTo(oldcontext); + MemoryContextReset(myState->tmpcontext); } /* ---------------- @@ -386,6 +387,7 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self) { TupleDesc typeinfo = slot->tts_tupleDescriptor; DR_printtup *myState = (DR_printtup *) self; + MemoryContext oldcontext; StringInfoData buf; int natts = typeinfo->natts; int i, @@ -399,6 +401,9 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self) /* Make sure the tuple is fully deconstructed */ slot_getallattrs(slot); + /* Switch into per-row context so we can recover memory below */ + oldcontext = MemoryContextSwitchTo(myState->tmpcontext); + /* * tell the frontend to expect new tuple data (in ASCII style) */ @@ -430,8 +435,7 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self) for (i = 0; i < natts; ++i) { PrinttupAttrInfo *thisState = myState->myinfo + i; - Datum origattr = slot->tts_values[i], - attr; + Datum attr = slot->tts_values[i]; char *outputstr; if (slot->tts_isnull[i]) @@ -439,25 +443,15 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self) Assert(thisState->format == 0); - /* - * If we have a toasted datum, forcibly detoast it here to avoid - * memory leakage inside the type's output routine. - */ - if (thisState->typisvarlena) - attr = PointerGetDatum(PG_DETOAST_DATUM(origattr)); - else - attr = origattr; - outputstr = OutputFunctionCall(&thisState->finfo, attr); pq_sendcountedtext(&buf, outputstr, strlen(outputstr), true); - pfree(outputstr); - - /* Clean up detoasted copy, if any */ - if (DatumGetPointer(attr) != DatumGetPointer(origattr)) - pfree(DatumGetPointer(attr)); } pq_endmessage(&buf); + + /* Return to caller's context, and flush row's temporary memory */ + MemoryContextSwitchTo(oldcontext); + MemoryContextReset(myState->tmpcontext); } /* ---------------- @@ -474,6 +468,10 @@ printtup_shutdown(DestReceiver *self) myState->myinfo = NULL; myState->attrinfo = NULL; + + if (myState->tmpcontext) + MemoryContextDelete(myState->tmpcontext); + myState->tmpcontext = NULL; } /* ---------------- @@ -536,8 +534,7 @@ debugtup(TupleTableSlot *slot, DestReceiver *self) TupleDesc typeinfo = slot->tts_tupleDescriptor; int natts = typeinfo->natts; int i; - Datum origattr, - attr; + Datum attr; char *value; bool isnull; Oid typoutput; @@ -545,30 +542,15 @@ debugtup(TupleTableSlot *slot, DestReceiver *self) for (i = 0; i < natts; ++i) { - origattr = slot_getattr(slot, i + 1, &isnull); + attr = slot_getattr(slot, i + 1, &isnull); if (isnull) continue; getTypeOutputInfo(typeinfo->attrs[i]->atttypid, &typoutput, &typisvarlena); - /* - * If we have a toasted datum, forcibly detoast it here to avoid - * memory leakage inside the type's output routine. - */ - if (typisvarlena) - attr = PointerGetDatum(PG_DETOAST_DATUM(origattr)); - else - attr = origattr; - value = OidOutputFunctionCall(typoutput, attr); printatt((unsigned) i + 1, typeinfo->attrs[i], value); - - pfree(value); - - /* Clean up detoasted copy, if any */ - if (DatumGetPointer(attr) != DatumGetPointer(origattr)) - pfree(DatumGetPointer(attr)); } printf("\t----\n"); } @@ -587,6 +569,7 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self) { TupleDesc typeinfo = slot->tts_tupleDescriptor; DR_printtup *myState = (DR_printtup *) self; + MemoryContext oldcontext; StringInfoData buf; int natts = typeinfo->natts; int i, @@ -600,6 +583,9 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self) /* Make sure the tuple is fully deconstructed */ slot_getallattrs(slot); + /* Switch into per-row context so we can recover memory below */ + oldcontext = MemoryContextSwitchTo(myState->tmpcontext); + /* * tell the frontend to expect new tuple data (in binary style) */ @@ -631,8 +617,7 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self) for (i = 0; i < natts; ++i) { PrinttupAttrInfo *thisState = myState->myinfo + i; - Datum origattr = slot->tts_values[i], - attr; + Datum attr = slot->tts_values[i]; bytea *outputbytes; if (slot->tts_isnull[i]) @@ -640,26 +625,15 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self) Assert(thisState->format == 1); - /* - * If we have a toasted datum, forcibly detoast it here to avoid - * memory leakage inside the type's output routine. - */ - if (thisState->typisvarlena) - attr = PointerGetDatum(PG_DETOAST_DATUM(origattr)); - else - attr = origattr; - outputbytes = SendFunctionCall(&thisState->finfo, attr); - /* We assume the result will not have been toasted */ pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4); pq_sendbytes(&buf, VARDATA(outputbytes), VARSIZE(outputbytes) - VARHDRSZ); - pfree(outputbytes); - - /* Clean up detoasted copy, if any */ - if (DatumGetPointer(attr) != DatumGetPointer(origattr)) - pfree(DatumGetPointer(attr)); } pq_endmessage(&buf); + + /* Return to caller's context, and flush row's temporary memory */ + MemoryContextSwitchTo(oldcontext); + MemoryContextReset(myState->tmpcontext); } diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 2a1af63d78..62f4c186f4 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -835,7 +835,6 @@ InsertOneValue(char *value, int i) Oid typioparam; Oid typinput; Oid typoutput; - char *prt; AssertArg(i >= 0 && i < MAXATTR); @@ -849,9 +848,14 @@ InsertOneValue(char *value, int i) &typinput, &typoutput); values[i] = OidInputFunctionCall(typinput, value, typioparam, -1); - prt = OidOutputFunctionCall(typoutput, values[i]); - elog(DEBUG4, "inserted -> %s", prt); - pfree(prt); + + /* + * We use ereport not elog here so that parameters aren't evaluated unless + * the message is going to be printed, which generally it isn't + */ + ereport(DEBUG4, + (errmsg_internal("inserted -> %s", + OidOutputFunctionCall(typoutput, values[i])))); } /* ---------------- diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index d91a663e31..dd352128d6 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -869,9 +869,7 @@ SPI_fname(TupleDesc tupdesc, int fnumber) char * SPI_getvalue(HeapTuple tuple, TupleDesc tupdesc, int fnumber) { - char *result; - Datum origval, - val; + Datum val; bool isnull; Oid typoid, foutoid; @@ -886,7 +884,7 @@ SPI_getvalue(HeapTuple tuple, TupleDesc tupdesc, int fnumber) return NULL; } - origval = heap_getattr(tuple, fnumber, tupdesc, &isnull); + val = heap_getattr(tuple, fnumber, tupdesc, &isnull); if (isnull) return NULL; @@ -897,22 +895,7 @@ SPI_getvalue(HeapTuple tuple, TupleDesc tupdesc, int fnumber) getTypeOutputInfo(typoid, &foutoid, &typisvarlena); - /* - * If we have a toasted datum, forcibly detoast it here to avoid memory - * leakage inside the type's output routine. - */ - if (typisvarlena) - val = PointerGetDatum(PG_DETOAST_DATUM(origval)); - else - val = origval; - - result = OidOutputFunctionCall(foutoid, val); - - /* Clean up detoasted copy, if any */ - if (val != origval) - pfree(DatumGetPointer(val)); - - return result; + return OidOutputFunctionCall(foutoid, val); } Datum diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c index cb04a724d3..33647f7d23 100644 --- a/src/backend/utils/adt/rowtypes.c +++ b/src/backend/utils/adt/rowtypes.c @@ -398,15 +398,7 @@ record_out(PG_FUNCTION_ARGS) column_info->column_type = column_type; } - /* - * If we have a toasted datum, forcibly detoast it here to avoid - * memory leakage inside the type's output routine. - */ - if (column_info->typisvarlena) - attr = PointerGetDatum(PG_DETOAST_DATUM(values[i])); - else - attr = values[i]; - + attr = values[i]; value = OutputFunctionCall(&column_info->proc, attr); /* Detect whether we need double quotes for this value */ @@ -437,12 +429,6 @@ record_out(PG_FUNCTION_ARGS) } if (nq) appendStringInfoCharMacro(&buf, '"'); - - pfree(value); - - /* Clean up detoasted copy, if any */ - if (DatumGetPointer(attr) != DatumGetPointer(values[i])) - pfree(DatumGetPointer(attr)); } appendStringInfoChar(&buf, ')'); @@ -759,27 +745,11 @@ record_send(PG_FUNCTION_ARGS) column_info->column_type = column_type; } - /* - * If we have a toasted datum, forcibly detoast it here to avoid - * memory leakage inside the type's output routine. - */ - if (column_info->typisvarlena) - attr = PointerGetDatum(PG_DETOAST_DATUM(values[i])); - else - attr = values[i]; - + attr = values[i]; outputbytes = SendFunctionCall(&column_info->proc, attr); - - /* We assume the result will not have been toasted */ pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4); pq_sendbytes(&buf, VARDATA(outputbytes), VARSIZE(outputbytes) - VARHDRSZ); - - pfree(outputbytes); - - /* Clean up detoasted copy, if any */ - if (DatumGetPointer(attr) != DatumGetPointer(values[i])) - pfree(DatumGetPointer(attr)); } pfree(values);