I looked into the problem complained of here:
http://archives.postgresql.org/pgsql-general/2012-11/msg00279.php
which turns out to have nothing to do with joins and everything to do
with the fact that record_out() leaks memory like mad.  It leaks both
the strings returned by the per-column output functions and any column
values that have to be detoasted.  You can easily reproduce this with
an example like

create table leak (f1 int, f2 text);

insert into leak select x, 'foo' from generate_series(1,1000000) x;

select leak from leak;

The attached patch against HEAD fixes this, as well as a similar
leakage in record_send().  The added code is lifted directly from
printtup() so it's not adding any new assumptions to the system.

I wonder though if we ought to think about running output functions in
a short-lived memory context instead of the executor's main context.
We've considered that before, I think, and it's always been the path
of least resistance to fix the output functions instead --- but there
will always be another leak I'm afraid.

OTOH I can't see trying to back-patch a solution like that.   If we want
to fix this in the back branches (and note the complaint linked above is
against 8.3), I think we have to do it as attached.

Thoughts?

                        regards, tom lane

diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c
index 13e574d4e82b4412f7ab32e02398d1671d381097..d4ed7d0ca06ac1a0dee6b4963989a5b41713fb3b 100644
*** a/src/backend/utils/adt/rowtypes.c
--- b/src/backend/utils/adt/rowtypes.c
*************** typedef struct ColumnIOData
*** 32,37 ****
--- 32,38 ----
  	Oid			column_type;
  	Oid			typiofunc;
  	Oid			typioparam;
+ 	bool		typisvarlena;
  	FmgrInfo	proc;
  } ColumnIOData;
  
*************** record_out(PG_FUNCTION_ARGS)
*** 364,369 ****
--- 365,371 ----
  	{
  		ColumnIOData *column_info = &my_extra->columns[i];
  		Oid			column_type = tupdesc->attrs[i]->atttypid;
+ 		Datum		attr;
  		char	   *value;
  		char	   *tmp;
  		bool		nq;
*************** record_out(PG_FUNCTION_ARGS)
*** 387,403 ****
  		 */
  		if (column_info->column_type != column_type)
  		{
- 			bool		typIsVarlena;
- 
  			getTypeOutputInfo(column_type,
  							  &column_info->typiofunc,
! 							  &typIsVarlena);
  			fmgr_info_cxt(column_info->typiofunc, &column_info->proc,
  						  fcinfo->flinfo->fn_mcxt);
  			column_info->column_type = column_type;
  		}
  
! 		value = OutputFunctionCall(&column_info->proc, values[i]);
  
  		/* Detect whether we need double quotes for this value */
  		nq = (value[0] == '\0');	/* force quotes for empty string */
--- 389,412 ----
  		 */
  		if (column_info->column_type != column_type)
  		{
  			getTypeOutputInfo(column_type,
  							  &column_info->typiofunc,
! 							  &column_info->typisvarlena);
  			fmgr_info_cxt(column_info->typiofunc, &column_info->proc,
  						  fcinfo->flinfo->fn_mcxt);
  			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];
! 
! 		value = OutputFunctionCall(&column_info->proc, attr);
  
  		/* Detect whether we need double quotes for this value */
  		nq = (value[0] == '\0');	/* force quotes for empty string */
*************** record_out(PG_FUNCTION_ARGS)
*** 416,432 ****
  
  		/* And emit the string */
  		if (nq)
! 			appendStringInfoChar(&buf, '"');
  		for (tmp = value; *tmp; tmp++)
  		{
  			char		ch = *tmp;
  
  			if (ch == '"' || ch == '\\')
! 				appendStringInfoChar(&buf, ch);
! 			appendStringInfoChar(&buf, ch);
  		}
  		if (nq)
! 			appendStringInfoChar(&buf, '"');
  	}
  
  	appendStringInfoChar(&buf, ')');
--- 425,447 ----
  
  		/* And emit the string */
  		if (nq)
! 			appendStringInfoCharMacro(&buf, '"');
  		for (tmp = value; *tmp; tmp++)
  		{
  			char		ch = *tmp;
  
  			if (ch == '"' || ch == '\\')
! 				appendStringInfoCharMacro(&buf, ch);
! 			appendStringInfoCharMacro(&buf, ch);
  		}
  		if (nq)
! 			appendStringInfoCharMacro(&buf, '"');
! 
! 		pfree(value);
! 
! 		/* Clean up detoasted copy, if any */
! 		if (DatumGetPointer(attr) != DatumGetPointer(values[i]))
! 			pfree(DatumGetPointer(attr));
  	}
  
  	appendStringInfoChar(&buf, ')');
*************** record_send(PG_FUNCTION_ARGS)
*** 714,719 ****
--- 729,735 ----
  	{
  		ColumnIOData *column_info = &my_extra->columns[i];
  		Oid			column_type = tupdesc->attrs[i]->atttypid;
+ 		Datum		attr;
  		bytea	   *outputbytes;
  
  		/* Ignore dropped columns in datatype */
*************** record_send(PG_FUNCTION_ARGS)
*** 734,756 ****
  		 */
  		if (column_info->column_type != column_type)
  		{
- 			bool		typIsVarlena;
- 
  			getTypeBinaryOutputInfo(column_type,
  									&column_info->typiofunc,
! 									&typIsVarlena);
  			fmgr_info_cxt(column_info->typiofunc, &column_info->proc,
  						  fcinfo->flinfo->fn_mcxt);
  			column_info->column_type = column_type;
  		}
  
! 		outputbytes = SendFunctionCall(&column_info->proc, values[i]);
  
  		/* 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);
  	}
  
  	pfree(values);
--- 750,784 ----
  		 */
  		if (column_info->column_type != column_type)
  		{
  			getTypeBinaryOutputInfo(column_type,
  									&column_info->typiofunc,
! 									&column_info->typisvarlena);
  			fmgr_info_cxt(column_info->typiofunc, &column_info->proc,
  						  fcinfo->flinfo->fn_mcxt);
  			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];
! 
! 		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);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to