Re: patch for large queries - Mailing list pgsql-hackers

I think it was agreed that this patch was too ugly to apply.  We have
addressed the aggregate memory case already.

> Hi Hackers,
> 
> I don't like last minute patches before the final freeze, but I believe that
> this one could be useful for people experiencing out-of-memory crashes while
> executing queries which retrieve or use a very large number of tuples.
> 
> The problem happens when storage is allocated for functions results used in
> a large query, for example:
> 
>   select upper(name) from big_table;
>   select big_table.array[1] from big_table;
>   select count(upper(name)) from big_table;
> 
> This patch is a dirty hack that fixes the out-of-memory problem for the most
> common cases, like the above ones. It is not the final solution for the
> problem but it can work for some people, so I'm posting it.
> 
> The patch should be safe because all changes are under #ifdef. Furthermore
> the feature can be enabled or disabled at runtime by the `free_tuple_memory'
> options in the pg_options file. The option is disabled by default and must
> be explicitly enabled at runtime to have any effect.
> 
> To enable the patch add the follwing line to Makefile.custom:
> 
> CUSTOM_COPT += -DFREE_TUPLE_MEMORY
> 
> To enable the option at runtime add the following line to pg_option:
> 
> free_tuple_memory=1
> 
> Here is the patch:
> 
> *** src/include/utils/portal.h.orig    Wed May 26 09:07:16 1999
> --- src/include/utils/portal.h    Fri Jun 11 18:06:07 1999
> ***************
> *** 80,85 ****
> --- 80,89 ----
>   extern PortalVariableMemory PortalGetVariableMemory(Portal portal);
>   extern PortalHeapMemory PortalGetHeapMemory(Portal portal);
>   
> + #ifdef FREE_TUPLE_MEMORY
> + bool   PortalHeapMemoryIsValid(MemoryContext context, Pointer pointer);
> + #endif
> + 
>   /* estimate of the maximum number of open portals a user would have,
>    * used in initially sizing the PortalHashTable in    EnablePortalManager()
>    */
> *** src/backend/utils/mmgr/portalmem.c.orig    Wed May 26 09:06:02 1999
> --- src/backend/utils/mmgr/portalmem.c    Fri Jun 11 18:06:28 1999
> ***************
> *** 289,294 ****
> --- 289,312 ----
>       }
>   }
>   
> + #ifdef FREE_TUPLE_MEMORY
> + /*
> +  * PortalHeapMemoryIsValid --
> +  *
> +  * Check if a pointer is allocated in a memory context.
> +  *
> +  */
> + bool
> + PortalHeapMemoryIsValid(MemoryContext context, Pointer pointer)
> + {
> +     HeapMemoryBlock block = HEAPMEMBLOCK((PortalHeapMemory) context);
> + 
> +     AssertState(PointerIsValid(block));
> + 
> +     return (AllocSetContains(&block->setData, pointer));
> + }
> + #endif
> + 
>   /* ----------------
>    *        PortalHeapMemoryRealloc
>    * ----------------
> *** src/include/utils/trace.h.orig    Sat Jun  5 09:00:40 1999
> --- src/include/utils/trace.h    Fri Jun 11 19:01:30 1999
> ***************
> *** 64,69 ****
> --- 64,72 ----
>       OPT_SYSLOG,                    /* use syslog for error messages */
>       OPT_HOSTLOOKUP,                /* enable hostname lookup in ps_status */
>       OPT_SHOWPORTNUMBER,            /* show port number in ps_status */
> + #ifdef FREE_TUPLE_MEMORY
> +     OPT_FREE_TUPLE_MEMORY,        /* try to pfree memory for each tuple */
> + #endif
>   
>       NUM_PG_OPTIONS                /* must be the last item of enum */
>   };
> ***************
> *** 83,91 ****
>   #endif     /* TRACE_H */
>   
>   /*
> !  * Local variables:
> !  *    tab-width: 4
> !  *    c-indent-level: 4
> !  *    c-basic-offset: 4
>    * End:
>    */
> --- 86,94 ----
>   #endif     /* TRACE_H */
>   
>   /*
> !  * Local Variables:
> !  *  tab-width: 4
> !  *  c-indent-level: 4
> !  *  c-basic-offset: 4
>    * End:
>    */
> *** src/backend/utils/misc/trace.c.orig    Sat Jun  5 09:00:37 1999
> --- src/backend/utils/misc/trace.c    Fri Jun 11 19:01:31 1999
> ***************
> *** 73,78 ****
> --- 73,81 ----
>       "syslog",                    /* use syslog for error messages */
>       "hostlookup",                /* enable hostname lookup in ps_status */
>       "showportnumber",            /* show port number in ps_status */
> + #ifdef FREE_TUPLE_MEMORY
> +     "free_tuple_memory",        /* try to pfree memory for each tuple */
> + #endif
>   
>       /* NUM_PG_OPTIONS */        /* must be the last item of enum */
>   };
> ***************
> *** 404,412 ****
>   }
>   
>   /*
> !  * Local variables:
> !  *    tab-width: 4
> !  *    c-indent-level: 4
> !  *    c-basic-offset: 4
>    * End:
>    */
> --- 407,415 ----
>   }
>   
>   /*
> !  * Local Variables:
> !  *  tab-width: 4
> !  *  c-indent-level: 4
> !  *  c-basic-offset: 4
>    * End:
>    */
> *** src/backend/access/common/heaptuple.c.orig    Wed May 26 09:01:59 1999
> --- src/backend/access/common/heaptuple.c    Fri Jun 11 19:08:33 1999
> ***************
> *** 27,32 ****
> --- 27,37 ----
>   #include <storage/bufpage.h>
>   #include <utils/memutils.h>
>   
> + #ifdef FREE_TUPLE_MEMORY
> + #include <utils/portal.h>
> + #include <utils/trace.h>
> + #endif
> + 
>   #ifndef HAVE_MEMMOVE
>   #include <regex/utils.h>
>   #else
> ***************
> *** 93,98 ****
> --- 98,106 ----
>       int            i;
>       int            numberOfAttributes = tupleDesc->natts;
>       Form_pg_attribute *att = tupleDesc->attrs;
> + #ifdef FREE_TUPLE_MEMORY
> +     bool        free_tuple_memory = pg_options[OPT_FREE_TUPLE_MEMORY];
> + #endif
>   
>       if (bit != NULL)
>       {
> ***************
> *** 131,136 ****
> --- 139,152 ----
>                   *infomask |= HEAP_HASVARLENA;
>                   data_length = VARSIZE(DatumGetPointer(value[i]));
>                   memmove(data, DatumGetPointer(value[i]), data_length);
> + #ifdef FREE_TUPLE_MEMORY
> +                 /* try to pfree value[i] - dz */
> +                 if (free_tuple_memory &&
> +                     PortalHeapMemoryIsValid(CurrentMemoryContext,
> +                                             (Pointer) value[i])) {
> +                     pfree(value[i]);
> +                 }
> + #endif
>                   break;
>               case sizeof(char):
>                   *data = att[i]->attbyval ?
> ***************
> *** 147,154 ****
>                                      *((int32 *) value[i]));
>                   break;
>               default:
> !                 memmove(data, DatumGetPointer(value[i]),
> !                         att[i]->attlen);
>                   break;
>           }
>           data = (char *) att_addlength((long) data, att[i]->attlen, value[i]);
> --- 163,177 ----
>                                      *((int32 *) value[i]));
>                   break;
>               default:
> !                 memmove(data, DatumGetPointer(value[i]), att[i]->attlen);
> ! #ifdef FREE_TUPLE_MEMORY
> !                 /* try to pfree value[i] - dz */
> !                 if (free_tuple_memory &&
> !                     PortalHeapMemoryIsValid(CurrentMemoryContext,
> !                                             (Pointer) value[i])) {
> !                     pfree(value[i]);
> !                 }
> ! #endif
>                   break;
>           }
>           data = (char *) att_addlength((long) data, att[i]->attlen, value[i]);
> *** src/backend/executor/nodeAgg.c.orig    Wed May 26 09:02:57 1999
> --- src/backend/executor/nodeAgg.c    Fri Jun 11 19:10:27 1999
> ***************
> *** 31,36 ****
> --- 31,41 ----
>   #include "utils/syscache.h"
>   #include "optimizer/clauses.h"
>   
> + #ifdef FREE_TUPLE_MEMORY
> + #include <utils/portal.h>
> + #include <utils/trace.h>
> + #endif
> + 
>   /*
>    * AggFuncInfo -
>    *      keeps the transition functions information around
> ***************
> *** 113,119 ****
>                   isNull1 = FALSE,
>                   isNull2 = FALSE;
>       bool        qual_result;
> ! 
>   
>       /* ---------------------
>        *    get state info from node
> --- 118,126 ----
>                   isNull1 = FALSE,
>                   isNull2 = FALSE;
>       bool        qual_result;
> ! #ifdef FREE_TUPLE_MEMORY
> !     bool        free_tuple_memory = pg_options[OPT_FREE_TUPLE_MEMORY];
> ! #endif
>   
>       /* ---------------------
>        *    get state info from node
> ***************
> *** 241,246 ****
> --- 248,257 ----
>           for (;;)
>           {
>               TupleTableSlot *outerslot;
> + #ifdef FREE_TUPLE_MEMORY
> +             Oid                valueType;
> +             bool            isByValue = 0;
> + #endif
>   
>               isNull = isNull1 = isNull2 = 0;
>               outerslot = ExecProcNode(outerPlan, (Plan *) node);
> ***************
> *** 293,298 ****
> --- 304,334 ----
>                       newVal = ExecEvalExpr(aggref->target, econtext,
>                                             &isNull, &isDone);
>                   }
> + #ifdef FREE_TUPLE_MEMORY
> +                 if (free_tuple_memory) {
> +                     switch (nodeTag(aggref->target)) {
> +                     case T_Const:
> +                         isByValue = ((Const*) (aggref->target))->constbyval;
> +                         break;
> +                     case T_Var:
> +                         valueType = ((Var*) (aggref->target))->vartype;
> +                         isByValue = typeByVal(typeidType(valueType));
> +                         break;
> +                     case T_Array:
> +                         isByValue = ((Array*)(aggref->target))->arrayelembyval;
> +                         break;
> +                     case T_ArrayRef:
> +                         isByValue =((ArrayRef*)(aggref->target))->refelembyval;
> +                         break;
> +                     case T_Expr:
> +                         valueType = ((Expr*) (aggref->target))->typeOid;
> +                         isByValue = typeByVal(typeidType(valueType));
> +                         break;
> +                     default:
> +                         break;
> +                     }
> +                 }
> + #endif
>   
>                   if (isNull && !aggref->usenulls)
>                       continue;    /* ignore this tuple for this agg */
> ***************
> *** 353,358 ****
> --- 389,404 ----
>                                             (FmgrValues *) args, &isNull2);
>                       Assert(!isNull2);
>                   }
> + 
> + #ifdef FREE_TUPLE_MEMORY
> +                 /* try to pfree newVal if not isByValue - dz */
> +                 if (free_tuple_memory && !isByValue && 
> +                     PortalHeapMemoryIsValid(CurrentMemoryContext,
> +                                             (Pointer) newVal))
> +                 {
> +                     pfree(newVal);
> +                 }
> + #endif
>               }
>   
>               /*
> 
> 
> -- 
> Massimo Dal Zotto
> 
> +----------------------------------------------------------------------+
> |  Massimo Dal Zotto               email: dz@cs.unitn.it               |
> |  Via Marconi, 141                phone: ++39-0461534251              |
> |  38057 Pergine Valsugana (TN)      www: http://www.cs.unitn.it/~dz/  |
> |  Italy                             pgp: finger dz@tango.cs.unitn.it  |
> +----------------------------------------------------------------------+
> 


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