Make tuple receive/print routines TOAST-aware. Formerly, printtup would
authorTom Lane
Fri, 1 Dec 2000 22:10:31 +0000 (22:10 +0000)
committerTom Lane
Fri, 1 Dec 2000 22:10:31 +0000 (22:10 +0000)
leak memory when printing a toasted attribute, and printtup_internal
didn't work at all...

src/backend/access/common/printtup.c
src/backend/executor/spi.c
src/backend/tcop/dest.c
src/include/access/printtup.h

index ccf3071b502d2cd8ba2866ead9d027fa23a47d1a..b05902068ee20b20f579dd87b83677303cdf9475 100644 (file)
@@ -9,12 +9,10 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/access/common/printtup.c,v 1.54 2000/11/16 22:30:15 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/access/common/printtup.c,v 1.55 2000/12/01 22:10:31 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
-
-
 #include "postgres.h"
 
 #include "access/heapam.h"
@@ -25,6 +23,7 @@
 
 static void printtup_setup(DestReceiver *self, TupleDesc typeinfo);
 static void printtup(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self);
+static void printtup_internal(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self);
 static void printtup_cleanup(DestReceiver *self);
 
 /* ----------------------------------------------------------------
@@ -33,15 +32,12 @@ static void printtup_cleanup(DestReceiver *self);
  */
 
 /* ----------------
- *     getTypeOutAndElem -- get both typoutput and typelem for a type
- *
- * We used to fetch these with two separate function calls,
- * typtoout() and gettypelem(), which each called SearchSysCache.
- * This way takes half the time.
+ *     getTypeOutputInfo -- get info needed for printing values of a type
  * ----------------
  */
-int
-getTypeOutAndElem(Oid type, Oid *typOutput, Oid *typElem)
+bool
+getTypeOutputInfo(Oid type, Oid *typOutput, Oid *typElem,
+                 bool *typIsVarlena)
 {
    HeapTuple   typeTuple;
    Form_pg_type pt;
@@ -50,11 +46,12 @@ getTypeOutAndElem(Oid type, Oid *typOutput, Oid *typElem)
                               ObjectIdGetDatum(type),
                               0, 0, 0);
    if (!HeapTupleIsValid(typeTuple))
-       elog(ERROR, "getTypeOutAndElem: Cache lookup of type %u failed", type);
+       elog(ERROR, "getTypeOutputInfo: Cache lookup of type %u failed", type);
    pt = (Form_pg_type) GETSTRUCT(typeTuple);
 
    *typOutput = pt->typoutput;
    *typElem = pt->typelem;
+   *typIsVarlena = (! pt->typbyval) && (pt->typlen == -1);
    ReleaseSysCache(typeTuple);
    return OidIsValid(*typOutput);
 }
@@ -67,6 +64,7 @@ typedef struct
 {                              /* Per-attribute information */
    Oid         typoutput;      /* Oid for the attribute's type output fn */
    Oid         typelem;        /* typelem value to pass to the output fn */
+   bool        typisvarlena;   /* is it varlena (ie possibly toastable)? */
    FmgrInfo    finfo;          /* Precomputed call info for typoutput */
 } PrinttupAttrInfo;
 
@@ -83,11 +81,11 @@ typedef struct
  * ----------------
  */
 DestReceiver *
-printtup_create_DR()
+printtup_create_DR(bool isBinary)
 {
    DR_printtup *self = (DR_printtup *) palloc(sizeof(DR_printtup));
 
-   self->pub.receiveTuple = printtup;
+   self->pub.receiveTuple = isBinary ? printtup_internal : printtup;
    self->pub.setup = printtup_setup;
    self->pub.cleanup = printtup_cleanup;
 
@@ -132,8 +130,9 @@ printtup_prepare_info(DR_printtup *myState, TupleDesc typeinfo, int numAttrs)
    {
        PrinttupAttrInfo *thisState = myState->myinfo + i;
 
-       if (getTypeOutAndElem((Oid) typeinfo->attrs[i]->atttypid,
-                             &thisState->typoutput, &thisState->typelem))
+       if (getTypeOutputInfo(typeinfo->attrs[i]->atttypid,
+                             &thisState->typoutput, &thisState->typelem,
+                             &thisState->typisvarlena))
            fmgr_info(thisState->typoutput, &thisState->finfo);
    }
 }
@@ -147,17 +146,14 @@ printtup(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self)
 {
    DR_printtup *myState = (DR_printtup *) self;
    StringInfoData buf;
+   int         natts = tuple->t_data->t_natts;
    int         i,
                j,
                k;
-   char       *outputstr;
-   Datum       attr;
-   bool        isnull;
 
    /* Set or update my derived attribute info, if needed */
-   if (myState->attrinfo != typeinfo ||
-       myState->nattrs != tuple->t_data->t_natts)
-       printtup_prepare_info(myState, typeinfo, tuple->t_data->t_natts);
+   if (myState->attrinfo != typeinfo || myState->nattrs != natts)
+       printtup_prepare_info(myState, typeinfo, natts);
 
    /* ----------------
     *  tell the frontend to expect new tuple data (in ASCII style)
@@ -172,7 +168,7 @@ printtup(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self)
     */
    j = 0;
    k = 1 << 7;
-   for (i = 0; i < tuple->t_data->t_natts; ++i)
+   for (i = 0; i < natts; ++i)
    {
        if (!heap_attisnull(tuple, i + 1))
            j |= k;             /* set bit if not null */
@@ -191,20 +187,38 @@ printtup(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self)
     *  send the attributes of this tuple
     * ----------------
     */
-   for (i = 0; i < tuple->t_data->t_natts; ++i)
+   for (i = 0; i < natts; ++i)
    {
        PrinttupAttrInfo *thisState = myState->myinfo + i;
+       Datum       origattr,
+                   attr;
+       bool        isnull;
+       char       *outputstr;
 
-       attr = heap_getattr(tuple, i + 1, typeinfo, &isnull);
+       origattr = heap_getattr(tuple, i + 1, typeinfo, &isnull);
        if (isnull)
            continue;
        if (OidIsValid(thisState->typoutput))
        {
+           /*
+            * 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 = DatumGetCString(FunctionCall3(&thisState->finfo,
                                        attr,
                                        ObjectIdGetDatum(thisState->typelem),
                                        Int32GetDatum(typeinfo->attrs[i]->atttypmod)));
+
            pq_sendcountedtext(&buf, outputstr, strlen(outputstr));
+
+           /* Clean up detoasted copy, if any */
+           if (attr != origattr)
+               pfree(DatumGetPointer(attr));
            pfree(outputstr);
        }
        else
@@ -276,26 +290,43 @@ showatts(char *name, TupleDesc tupleDesc)
 void
 debugtup(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self)
 {
+   int         natts = tuple->t_data->t_natts;
    int         i;
-   Datum       attr;
+   Datum       origattr,
+               attr;
    char       *value;
    bool        isnull;
    Oid         typoutput,
                typelem;
+   bool        typisvarlena;
 
-   for (i = 0; i < tuple->t_data->t_natts; ++i)
+   for (i = 0; i < natts; ++i)
    {
-       attr = heap_getattr(tuple, i + 1, typeinfo, &isnull);
+       origattr = heap_getattr(tuple, i + 1, typeinfo, &isnull);
        if (isnull)
            continue;
-       if (getTypeOutAndElem((Oid) typeinfo->attrs[i]->atttypid,
-                             &typoutput, &typelem))
+       if (getTypeOutputInfo(typeinfo->attrs[i]->atttypid,
+                             &typoutput, &typelem, &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 = DatumGetCString(OidFunctionCall3(typoutput,
                                    attr,
                                    ObjectIdGetDatum(typelem),
                                    Int32GetDatum(typeinfo->attrs[i]->atttypmod)));
+
            printatt((unsigned) i + 1, typeinfo->attrs[i], value);
+
+           /* Clean up detoasted copy, if any */
+           if (attr != origattr)
+               pfree(DatumGetPointer(attr));
            pfree(value);
        }
    }
@@ -307,19 +338,22 @@ debugtup(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self)
  *     We use a different data prefix, e.g. 'B' instead of 'D' to
  *     indicate a tuple in internal (binary) form.
  *
- *     This is same as printtup, except we don't use the typout func,
- *     and therefore have no need for persistent state.
+ *     This is largely same as printtup, except we don't use the typout func.
  * ----------------
  */
-void
+static void
 printtup_internal(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self)
 {
+   DR_printtup *myState = (DR_printtup *) self;
    StringInfoData buf;
+   int         natts = tuple->t_data->t_natts;
    int         i,
                j,
                k;
-   Datum       attr;
-   bool        isnull;
+
+   /* Set or update my derived attribute info, if needed */
+   if (myState->attrinfo != typeinfo || myState->nattrs != natts)
+       printtup_prepare_info(myState, typeinfo, natts);
 
    /* ----------------
     *  tell the frontend to expect new tuple data (in binary style)
@@ -334,7 +368,7 @@ printtup_internal(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self)
     */
    j = 0;
    k = 1 << 7;
-   for (i = 0; i < tuple->t_data->t_natts; ++i)
+   for (i = 0; i < natts; ++i)
    {
        if (!heap_attisnull(tuple, i + 1))
            j |= k;             /* set bit if not null */
@@ -354,71 +388,87 @@ printtup_internal(HeapTuple tuple, TupleDesc typeinfo, DestReceiver *self)
     * ----------------
     */
 #ifdef IPORTAL_DEBUG
-   fprintf(stderr, "sending tuple with %d atts\n", tuple->t_data->t_natts);
+   fprintf(stderr, "sending tuple with %d atts\n", natts);
 #endif
-   for (i = 0; i < tuple->t_data->t_natts; ++i)
+
+   for (i = 0; i < natts; ++i)
    {
-       int32       len = typeinfo->attrs[i]->attlen;
+       PrinttupAttrInfo *thisState = myState->myinfo + i;
+       Datum       origattr,
+                   attr;
+       bool        isnull;
+       int32       len;
 
-       attr = heap_getattr(tuple, i + 1, typeinfo, &isnull);
-       if (!isnull)
+       origattr = heap_getattr(tuple, i + 1, typeinfo, &isnull);
+       if (isnull)
+           continue;
+       /* send # of bytes, and opaque data */
+       if (thisState->typisvarlena)
        {
-           /* # of bytes, and opaque data */
-           if (len == -1)
-           {
-               /* variable length, assume a varlena structure */
-               len = VARSIZE(attr) - VARHDRSZ;
+           /*
+            * If we have a toasted datum, must detoast before sending.
+            */
+           attr = PointerGetDatum(PG_DETOAST_DATUM(origattr));
+
+           len = VARSIZE(attr) - VARHDRSZ;
 
-               pq_sendint(&buf, len, VARHDRSZ);
-               pq_sendbytes(&buf, VARDATA(attr), len);
+           pq_sendint(&buf, len, VARHDRSZ);
+           pq_sendbytes(&buf, VARDATA(attr), len);
 
 #ifdef IPORTAL_DEBUG
-               {
-                   char       *d = VARDATA(attr);
+           {
+               char       *d = VARDATA(attr);
 
-                   fprintf(stderr, "length %d data %x%x%x%x\n",
-                           len, *d, *(d + 1), *(d + 2), *(d + 3));
-               }
-#endif
+               fprintf(stderr, "length %d data %x %x %x %x\n",
+                       len, *d, *(d + 1), *(d + 2), *(d + 3));
            }
-           else
+#endif
+
+           /* Clean up detoasted copy, if any */
+           if (attr != origattr)
+               pfree(DatumGetPointer(attr));
+       }
+       else
+       {
+           /* fixed size */
+           attr = origattr;
+           len = typeinfo->attrs[i]->attlen;
+           pq_sendint(&buf, len, sizeof(int32));
+           if (typeinfo->attrs[i]->attbyval)
            {
-               /* fixed size */
-               if (typeinfo->attrs[i]->attbyval)
+               int8        i8;
+               int16       i16;
+               int32       i32;
+
+               switch (len)
                {
-                   int8        i8;
-                   int16       i16;
-                   int32       i32;
-
-                   pq_sendint(&buf, len, sizeof(int32));
-                   switch (len)
-                   {
-                       case sizeof(int8):
-                           i8 = DatumGetChar(attr);
-                           pq_sendbytes(&buf, (char *) &i8, len);
-                           break;
-                       case sizeof(int16):
-                           i16 = DatumGetInt16(attr);
-                           pq_sendbytes(&buf, (char *) &i16, len);
-                           break;
-                       case sizeof(int32):
-                           i32 = DatumGetInt32(attr);
-                           pq_sendbytes(&buf, (char *) &i32, len);
-                           break;
-                   }
+                   case sizeof(int8):
+                       i8 = DatumGetChar(attr);
+                       pq_sendbytes(&buf, (char *) &i8, len);
+                       break;
+                   case sizeof(int16):
+                       i16 = DatumGetInt16(attr);
+                       pq_sendbytes(&buf, (char *) &i16, len);
+                       break;
+                   case sizeof(int32):
+                       i32 = DatumGetInt32(attr);
+                       pq_sendbytes(&buf, (char *) &i32, len);
+                       break;
+                   default:
+                       elog(ERROR, "printtup_internal: unexpected typlen");
+                       break;
+               }
 #ifdef IPORTAL_DEBUG
-                   fprintf(stderr, "byval length %d data %d\n", len, attr);
+               fprintf(stderr, "byval length %d data %d\n", len, attr);
 #endif
-               }
-               else
-               {
-                   pq_sendint(&buf, len, sizeof(int32));
-                   pq_sendbytes(&buf, DatumGetPointer(attr), len);
+           }
+           else
+           {
+               pq_sendbytes(&buf, DatumGetPointer(attr), len);
 #ifdef IPORTAL_DEBUG
-                   fprintf(stderr, "byref length %d data %x\n", len,
-                           DatumGetPointer(attr));
+               fprintf(stderr, "byref length %d data %p\n", len,
+                       DatumGetPointer(attr));
 #endif
-               }
            }
        }
    }
index 74a5dc44415fb05b9a758dd0f9243e4a9a056808..b309dc5022eddcd9b25284f4624e7300c98d140e 100644 (file)
@@ -3,7 +3,7 @@
  * spi.c
  *             Server Programming Interface
  *
- * $Id: spi.c,v 1.49 2000/11/16 22:30:22 tgl Exp $
+ * $Id: spi.c,v 1.50 2000/12/01 22:10:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -397,10 +397,13 @@ SPI_fname(TupleDesc tupdesc, int fnumber)
 char *
 SPI_getvalue(HeapTuple tuple, TupleDesc tupdesc, int fnumber)
 {
-   Datum       val;
+   Datum       origval,
+               val,
+               result;
    bool        isnull;
    Oid         foutoid,
                typelem;
+   bool        typisvarlena;
 
    SPI_result = 0;
    if (tuple->t_data->t_natts < fnumber || fnumber <= 0)
@@ -409,20 +412,35 @@ SPI_getvalue(HeapTuple tuple, TupleDesc tupdesc, int fnumber)
        return NULL;
    }
 
-   val = heap_getattr(tuple, fnumber, tupdesc, &isnull);
+   origval = heap_getattr(tuple, fnumber, tupdesc, &isnull);
    if (isnull)
        return NULL;
-   if (!getTypeOutAndElem((Oid) tupdesc->attrs[fnumber - 1]->atttypid,
-                          &foutoid, &typelem))
+   if (!getTypeOutputInfo(tupdesc->attrs[fnumber - 1]->atttypid,
+                          &foutoid, &typelem, &typisvarlena))
    {
        SPI_result = SPI_ERROR_NOOUTFUNC;
        return NULL;
    }
 
-   return DatumGetCString(OidFunctionCall3(foutoid,
-                          val,
-                          ObjectIdGetDatum(typelem),
-                          Int32GetDatum(tupdesc->attrs[fnumber - 1]->atttypmod)));
+   /*
+    * 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 = OidFunctionCall3(foutoid,
+                             val,
+                             ObjectIdGetDatum(typelem),
+                             Int32GetDatum(tupdesc->attrs[fnumber - 1]->atttypmod));
+
+   /* Clean up detoasted copy, if any */
+   if (val != origval)
+       pfree(DatumGetPointer(val));
+
+   return DatumGetCString(result);
 }
 
 Datum
index ad3475ba8a40b9058ba2b00e0fe10f5fb0b1747b..994c71c5a7d6a0a08c92e94376ec9100f54ae9e8 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/tcop/dest.c,v 1.40 2000/10/22 22:14:55 petere Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/tcop/dest.c,v 1.41 2000/12/01 22:10:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -69,9 +69,6 @@ donothingCleanup(DestReceiver *self)
 static DestReceiver donothingDR = {
    donothingReceive, donothingSetup, donothingCleanup
 };
-static DestReceiver printtup_internalDR = {
-   printtup_internal, donothingSetup, donothingCleanup
-};
 static DestReceiver debugtupDR = {
    debugtup, donothingSetup, donothingCleanup
 };
@@ -180,11 +177,10 @@ DestToFunction(CommandDest dest)
    switch (dest)
    {
        case Remote:
-       /* printtup wants a dynamically allocated DestReceiver */
-           return printtup_create_DR();
+           return printtup_create_DR(false);
 
        case RemoteInternal:
-           return &printtup_internalDR;
+           return printtup_create_DR(true);
 
        case Debug:
            return &debugtupDR;
index bd5acd13e75a80ef548a79d3bdedb5d6d85ccf05..a70a9c35e93f7b0b4a704bf11ab1253164dff6f0 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: printtup.h,v 1.12 2000/01/26 05:57:50 momjian Exp $
+ * $Id: printtup.h,v 1.13 2000/12/01 22:10:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 
 #include "tcop/dest.h"
 
-extern DestReceiver *printtup_create_DR(void);
+extern DestReceiver *printtup_create_DR(bool isBinary);
+
 extern void showatts(char *name, TupleDesc attinfo);
 extern void debugtup(HeapTuple tuple, TupleDesc typeinfo,
-        DestReceiver *self);
-extern void printtup_internal(HeapTuple tuple, TupleDesc typeinfo,
-                 DestReceiver *self);
+                    DestReceiver *self);
 
 /* XXX this one is really in executor/spi.c */
 extern void spi_printtup(HeapTuple tuple, TupleDesc tupdesc,
-            DestReceiver *self);
+                        DestReceiver *self);
 
-extern int getTypeOutAndElem(Oid type, Oid *typOutput, Oid *typElem);
+extern bool getTypeOutputInfo(Oid type, Oid *typOutput, Oid *typElem,
+                             bool *typIsVarlena);
 
 #endif  /* PRINTTUP_H */