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 PGresult finally. So I think it is preferable to use pqResultAlloc() or its descendents in consistensy with the nature of the 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 the four 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 for PGresult 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 int PQsendQueryGuts(PGconn *conn,
const char *command,
@@ -186,7 +186,7 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status)
/* copy events last; result must be valid if 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.
*/
static PGEvent *
-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 157
PQping 158
PQpingParams 159
PQlibVersion 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);
static int 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)
return NULL;
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 structure itself */
- 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
* ----------------
*/
testlibpq.c.gz
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
