Allow substitute allocators for PGresult. - Mailing list pgsql-hackers

Hello. This message is a proposal of a pair of patches that
enables the memory allocator for PGresult in libpq to be
replaced.


The comment at the the begging of pqexpbuffer.c says that libpq
should not rely on palloc(). Besides, Tom Lane said that palloc
should not be visible outside the backend(*1) and I agree with
it.

*1: http://archives.postgresql.org/pgsql-hackers/1999-02/msg00364.php

On the other hand, in dblink, dblink-plus (our product!), and
maybe FDW's connect to other PostgreSQL servers are seem to copy
the result of the query contained in PGresult into tuple store. I
guess that this is in order to avoid memory leakage on
termination in halfway.

But it is rather expensive to copy whole PGresult, and the
significance grows as the data received gets larger. Furthermore,
it requires about twice as much memory as the net size of the
data. And it is fruitless to copy'n modify libpq or reinvent it
from scratch. So we shall be happy to be able to use palloc's in
libpq at least for PGresult for such case in spite of the policy.


For these reasons, I propose to make allocators for PGresult
replaceable.

The modifications are made up into two patches.

1. dupEvents() and pqAddTuple() get new memory block by malloc  currently, but the aquired memory block is linked into
PGresultfinally. So I think it is preferable to use  pqResultAlloc() or its descendents in consistensy with the  nature
ofthe place to link.
 
  But there is not PQresultRealloc() and it will be costly, so  pqAddTuple() is not modified in this patch.


2. Define three function pointers  PQpgresult_(malloc|realloc|free) and replace the calls to  malloc/realloc/free in
thefour functions below with these  pointers.
 
  PQmakeEmptyPGresult()  pqResultAlloc()  PQclear()  pqAddTuple()

This patches make the tools run in backend process and use libpq
possible to handle PGresult as it is with no copy, no more memory.

(Of cource, someone wants to use his/her custom allocator forPGresult on standalone tools could do that using this
feature.)



Three files are attached to this message.

First, the patch with respect to "1" above.
Second, the patch with respect to "2" above.
Third, a very simple sample program.

I have built and briefly tested on CentOS6, with the sample
program mentioned above and valgrind, but not on Windows.


How do you think about this?

Regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 113aab0..8e32b18 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -49,7 +49,7 @@ static int    static_client_encoding = PG_SQL_ASCII;static bool static_std_strings = false;
-static PGEvent *dupEvents(PGEvent *events, int count);
+static PGEvent *dupEvents(PGresult *res, PGEvent *events, int count);static bool PQsendQueryStart(PGconn *conn);static
intPQsendQueryGuts(PGconn *conn,                const char *command,
 
@@ -186,7 +186,7 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)        /* copy events last; result must be
validif we need to PQclear */        if (conn->nEvents > 0)        {
 
-            result->events = dupEvents(conn->events, conn->nEvents);
+            result->events = dupEvents(result, conn->events, conn->nEvents);            if (!result->events)
{                PQclear(result);
 
@@ -337,7 +337,7 @@ PQcopyResult(const PGresult *src, int flags)    /* Wants to copy PGEvents? */    if ((flags &
PG_COPYRES_EVENTS)&& src->nEvents > 0)    {
 
-        dest->events = dupEvents(src->events, src->nEvents);
+        dest->events = dupEvents(dest, dest->events, src->nEvents);        if (!dest->events)        {
PQclear(dest);
@@ -374,7 +374,7 @@ PQcopyResult(const PGresult *src, int flags) * Also, the resultInitialized flags are all cleared.
*/staticPGEvent *
 
-dupEvents(PGEvent *events, int count)
+dupEvents(PGresult *res, PGEvent *events, int count){    PGEvent    *newEvents;    int            i;
@@ -382,7 +382,7 @@ dupEvents(PGEvent *events, int count)    if (!events || count <= 0)        return NULL;
-    newEvents = (PGEvent *) malloc(count * sizeof(PGEvent));
+    newEvents = (PGEvent *) pqResultAlloc(res, count * sizeof(PGEvent), TRUE);    if (!newEvents)        return NULL;
@@ -392,14 +392,9 @@ dupEvents(PGEvent *events, int count)        newEvents[i].passThrough = events[i].passThrough;
  newEvents[i].data = NULL;        newEvents[i].resultInitialized = FALSE;
 
-        newEvents[i].name = strdup(events[i].name);
+        newEvents[i].name = pqResultStrdup(res, events[i].name);        if (!newEvents[i].name)
-        {
-            while (--i >= 0)
-                free(newEvents[i].name);
-            free(newEvents);            return NULL;
-        }    }    return newEvents;
@@ -661,12 +656,8 @@ PQclear(PGresult *res)            (void) res->events[i].proc(PGEVT_RESULTDESTROY, &evt,
                          res->events[i].passThrough);        }
 
-        free(res->events[i].name);    }
-    if (res->events)
-        free(res->events);
-    /* Free all the subsidiary blocks */    while ((block = res->curBlock) != NULL)    {
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 1af8df6..3b26c7c 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -160,3 +160,6 @@ PQconnectStartParams      157PQping                    158PQpingParams              159PQlibVersion
            160
 
+PQpgresult_malloc      161
+PQpgresult_realloc      162
+PQpgresult_free          163
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 8e32b18..a574848 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -67,6 +67,15 @@ static int PQsendDescribe(PGconn *conn, char desc_type,               const char
*desc_target);staticint    check_field_number(const PGresult *res, int field_num);
 
+/* ---
+ * malloc/realloc/free for PGResult is replasable for in-backend use
+ * Note that the events having the event id PGEVT_RESULTDESTROY won't
+ * fire when you free the memory blocks for PGresult without
+ * PQclear().
+ */
+void *(*PQpgresult_malloc)(size_t size) = malloc;
+void *(*PQpgresult_realloc)(void *ptr, size_t size) = realloc;
+void (*PQpgresult_free)(void *ptr) = free;/* ---------------- * Space management for PGresult.
@@ -138,7 +147,7 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status){    PGresult   *result;
-    result = (PGresult *) malloc(sizeof(PGresult));
+    result = (PGresult *) PQpgresult_malloc(sizeof(PGresult));    if (!result)        return NULL;
@@ -536,7 +545,8 @@ pqResultAlloc(PGresult *res, size_t nBytes, bool isBinary)     */    if (nBytes >=
PGRESULT_SEP_ALLOC_THRESHOLD)   {
 
-        block = (PGresult_data *) malloc(nBytes + PGRESULT_BLOCK_OVERHEAD);
+        block =
+            (PGresult_data *) PQpgresult_malloc(nBytes + PGRESULT_BLOCK_OVERHEAD);        if (!block)
returnNULL;        space = block->space + PGRESULT_BLOCK_OVERHEAD;
 
@@ -560,7 +570,7 @@ pqResultAlloc(PGresult *res, size_t nBytes, bool isBinary)    }    /* Otherwise, start a new block.
*/
-    block = (PGresult_data *) malloc(PGRESULT_DATA_BLOCKSIZE);
+    block = (PGresult_data *) PQpgresult_malloc(PGRESULT_DATA_BLOCKSIZE);    if (!block)        return NULL;
block->next= res->curBlock;
 
@@ -662,12 +672,12 @@ PQclear(PGresult *res)    while ((block = res->curBlock) != NULL)    {        res->curBlock =
block->next;
-        free(block);
+        PQpgresult_free(block);    }    /* Free the top-level tuple pointer array */    if (res->tuples)
-        free(res->tuples);
+        PQpgresult_free(res->tuples);    /* zero out the pointer fields to catch programming errors */
res->attDescs= NULL;
 
@@ -679,7 +689,7 @@ PQclear(PGresult *res)    /* res->curBlock was zeroed out earlier */    /* Free the PGresult
structureitself */
 
-    free(res);
+    PQpgresult_free(res);}/*
@@ -844,10 +854,11 @@ pqAddTuple(PGresult *res, PGresAttValue *tup)        if (res->tuples == NULL)
newTuples= (PGresAttValue **)
 
-                malloc(newSize * sizeof(PGresAttValue *));
+                PQpgresult_malloc(newSize * sizeof(PGresAttValue *));        else            newTuples =
(PGresAttValue**)
 
-                realloc(res->tuples, newSize * sizeof(PGresAttValue *));
+                PQpgresult_realloc(res->tuples,
+                                 newSize * sizeof(PGresAttValue *));        if (!newTuples)            return FALSE;
    /* malloc or realloc failed */        res->tupArrSize = newSize;
 
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index d13a5b9..c958df1 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -226,6 +226,14 @@ typedef struct pgresAttDesc} PGresAttDesc;/* ----------------
+ * malloc/realloc/free for PGResult is replasable for in-backend use
+ * ----------------
+ */
+extern void *(*PQpgresult_malloc)(size_t size);
+extern void *(*PQpgresult_realloc)(void *ptr, size_t size);
+extern void (*PQpgresult_free)(void *ptr);
+
+/* ---------------- * Exported functions of libpq * ---------------- */