Обсуждение: libpq patch for binding tuples in a result set to user allocated memory

Having been used to being able to bind tuple data from a result set to
user allocated memory (rather than having to copy the data out of the
"libpq" like API's buffers to my memory locations) in my days working
with ODBC, Oracle & SQLServer's C-API, i created the following patch
which i thought i'd submit. I've included a README as well.

if anyone has a second it would be great to know if i'm doing something
stupid or if there's anything i could do to get this patch to libpq in
the mainline releases.

thanks.
jr

FROM THE README:

POSTGRESQL BIND PATCH FOR POSTGRESQL VERSION 6.5.3

1). INTRODUCTION

anyone interested in a very simple binding API for PGSQL-libpq that has very little
impact on the libpq source code should read on.

the API is accessed through the following two functions:

  extern void PQsetResultDest(PGconn *conn, PGresAttValue* dest);
  extern void PQclearResultDest(PGconn *conn);

which are found in libpq-fe.h once the patch is applied.

2). HOW DO I USE THESE FUNCTIONS?

use libpq as normal, however when you want to bind the columns of a result set to
specific memory locations...

  a). construct an array of PGresAttValue's whose size is equal to the number of
      columns in the result set. if you don't you'll core dump!

    i.e. for "select id, name from people"
         >> PGresAttValue bind_vec[2];

  b). fill out the vector with the binding info. specifically each PGresAttValue
      must have a valid "value" ptr of the desired destination address, and
      and "len" that is equal to or bigger than the length of the column that will
      be returned.

  c). immediately before calling PQexec on a "FETCH FORWARD 1" sql statement call
         >> PQsetResultDest(conn, bind_vec);

  d). immediately after PQexec returns, call
         >> PQclearResultDest(conn);

  e). that's it. now the results of the fetch are in the memory locations
      you set up in your PGresAttValue array.

3). EXTRA INFO

  a). if (PGresAttValue[i].len > column[i].len) then this patch will append a null
      terminator to the value. this happens to be very convenient when using
      strings.

  b). if (0==PGresAttValue[i].len) for any column i, then that column will
      not be bound but will be accessible through standard libpq API.

4). BIGGER CODE SAMPLE

here's a more in depth code sample for a interface layer we have that sits on top
of libpq (it uses the stdc++ library vector for the bind_vec shown above, and doesn't show step two which happens
elsewhere)... 

bool CCursorPGSql::fetch() throw (CDbError)
{
  // build sql
  tchar cmd[128];
  if (-1==snprintf(cmd, sizeof(cmd), "FETCH FORWARD 1 IN %s", cursor_name))
  {
    CDbError err;
    snprintf(err.message, sizeof(err.message),
    _text("fetch: cmd buffer too short"));
    throw err;
  }

  // setup bind locations
  if (bind_vector.size()>0)
    PQsetResultDest(db->postgres_handle(), bind_vector.begin());

  // execute it
  pg::result res(PQexec(db->postgres_handle(), cmd));
  const int rc = PQresultStatus(res);
  if (PGRES_BAD_RESPONSE==rc || PGRES_NONFATAL_ERROR==rc || PGRES_FATAL_ERROR==rc)
THROW_DBERROR(db->postgres_handle());

  // clear bindings on connection
  if (bind_vector.size()>0)
    PQclearResultDest(db->postgres_handle());

  // return code means any data returned?
  return (PQntuples(res));
}

and the patch itself (against 6.5.2 but cleanly applies to 6.5.3)

diff -u postgresql-6.5.2/src/interfaces/libpq/fe-exec.c postgresql-6.5.2-with-bind/src/interfaces/libpq/fe-exec.c
--- postgresql-6.5.2/src/interfaces/libpq/fe-exec.c    Thu May 27 21:54:53 1999
+++ postgresql-6.5.2-with-bind/src/interfaces/libpq/fe-exec.c    Thu Oct 21 15:32:28 1999
@@ -869,19 +869,53 @@
                 vlen = vlen - 4;
             if (vlen < 0)
                 vlen = 0;
-            if (tup[i].value == NULL)
-            {
+            if ((conn->tuple_destinations == NULL) ||
+                (0==conn->tuple_destinations[i].len_max))
+              {
+
+                if (tup[i].value == NULL)
+                  {
                 tup[i].value = (char *) pqResultAlloc(result, vlen + 1, binary);
                 if (tup[i].value == NULL)
-                    goto outOfMemory;
-            }
-            tup[i].len = vlen;
-            /* read in the value */
-            if (vlen > 0)
-                if (pqGetnchar((char *) (tup[i].value), vlen, conn))
+                  goto outOfMemory;
+                  }
+
+                /* read in the value */
+                if (vlen > 0)
+                  if (pqGetnchar((char *) (tup[i].value), vlen, conn))
+                return EOF;
+                /* we have to terminate this ourselves */
+                tup[i].value[vlen] = '\0';
+              }
+            else
+              {
+                if (conn->tuple_destinations[i].len_max < vlen)
+                {
+                    pqClearAsyncResult(conn);
+                    sprintf(conn->errorMessage,
+                            "getAnotherTuple() -- column %d is %d bytes larger than bound destination\n", i,
vlen-conn->tuple_destinations[i].len_max);
+                    conn->result = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR);
+                    conn->asyncStatus = PGASYNC_READY;
+                    /* Discard the broken message */
+                    conn->inStart = conn->inEnd;
                     return EOF;
-            /* we have to terminate this ourselves */
-            tup[i].value[vlen] = '\0';
+                }
+
+                /* we set length returned no matter what */
+                    *(conn->tuple_destinations[i].len_returned) = vlen;
+
+                /* read in the value */
+                if (vlen > 0)
+                {
+                  if (pqGetnchar((char *) (conn->tuple_destinations[i].value), vlen, conn))
+                    return EOF;
+
+                  /* we only null terminate when there's space */
+                  if (conn->tuple_destinations[i].len_max > vlen)
+                    conn->tuple_destinations[i].value[vlen] = '\0';
+                }
+              }
+            tup[i].len = vlen;
         }
         /* advance the bitmap stuff */
         bitcnt++;
@@ -1921,4 +1955,18 @@
         return 1;
     else
         return 0;
+}
+
+void
+PQsetResultDest(PGconn* conn, PGbinding* _dest)
+{
+  if (0==conn) return;
+  conn->tuple_destinations = _dest;
+}
+
+void
+PQclearResultDest(PGconn* conn)
+{
+  if (0==conn) return;
+  conn->tuple_destinations = 0;
 }
diff -u postgresql-6.5.2/src/interfaces/libpq/libpq-fe.h postgresql-6.5.2-with-bind/src/interfaces/libpq/libpq-fe.h
--- postgresql-6.5.2/src/interfaces/libpq/libpq-fe.h    Tue May 25 12:15:13 1999
+++ postgresql-6.5.2-with-bind/src/interfaces/libpq/libpq-fe.h    Thu Oct 21 15:37:04 1999
@@ -27,6 +27,13 @@

 /* Application-visible enum types */

+    typedef struct pgbinding
+    {
+      int len_max;        /* [IN] length in bytes of the value buffer */
+      int* len_returned; /* [OUT] pointer to int that receives bytes returned */
+      char* value;    /* [OUT] actual value returned */
+    } PGbinding;
+
     typedef enum
     {
         CONNECTION_OK,
@@ -198,6 +205,10 @@
                                                  void *arg);

 /* === in fe-exec.c === */
+
+    /* result destinationn functions (for column binding and other things...)*/
+    extern void PQsetResultDest(PGconn *conn, PGbinding* dest);
+    extern void PQclearResultDest(PGconn *conn);

     /* Simple synchronous query */
     extern PGresult *PQexec(PGconn *conn, const char *query);
Only in postgresql-6.5.2-with-bind/src/interfaces/libpq: libpq-fe.h~
diff -u postgresql-6.5.2/src/interfaces/libpq/libpq-int.h postgresql-6.5.2-with-bind/src/interfaces/libpq/libpq-int.h
--- postgresql-6.5.2/src/interfaces/libpq/libpq-int.h    Tue May 25 18:43:49 1999
+++ postgresql-6.5.2-with-bind/src/interfaces/libpq/libpq-int.h    Thu Oct 21 15:34:36 1999
@@ -217,6 +217,9 @@
     PGresult   *result;            /* result being constructed */
     PGresAttValue *curTuple;    /* tuple currently being read */

+                        /* optional column bind location */
+                        PGbinding *tuple_destinations;
+
     /* Message space.  Placed last for code-size reasons. */
     char        errorMessage[ERROR_MSG_LENGTH];
 };

------------------------------------------------------------------------
Joel W. Reed                              http://ruby.ddiworld.com/jreed
----------------We're lost, but we're making good time.----------------

Joel Reed <jreed@support.ddiworld.com> writes:
> Having been used to being able to bind tuple data from a result set to
> user allocated memory (rather than having to copy the data out of the
> "libpq" like API's buffers to my memory locations) in my days working
> with ODBC, Oracle & SQLServer's C-API, i created the following patch
> which i thought i'd submit. I've included a README as well. 

I do not like this idea very much --- in the first place, it ties the
application far too tightly to internal representational details that
should be private to libpq, and in the second place the implementation
you offer is full of little gotchas like

>   a). construct an array of PGresAttValue's whose size is equal to the
>       number of columns in the result set. if you don't you'll core dump!

I really doubt that avoiding PQgetvalue() calls is worth taking those
kinds of risks and narrowing our options for future reimplementation of
the library and protocol.

In the long run we'll probably be moving to some more-modern interface
design like CORBA, which should help to address performance concerns
like these.
        regards, tom lane

On Feb 14, tgl@sss.pgh.pa.us hacked the bitstream to say...
Tom> Joel Reed <jreed@support.ddiworld.com> writes:
Tom> > Having been used to being able to bind tuple data from a result set to
Tom> > user allocated memory (rather than having to copy the data out of the
Tom> > "libpq" like API's buffers to my memory locations) in my days working
Tom> > with ODBC, Oracle & SQLServer's C-API, i created the following patch
Tom> > which i thought i'd submit. I've included a README as well. 
Tom> 
Tom> I do not like this idea very much --- in the first place, it ties the
Tom> application far too tightly to internal representational details that
Tom> should be private to libpq, and in the second place the implementation
Tom> you offer is full of little gotchas like
Tom> 
Tom> >   a). construct an array of PGresAttValue's whose size is equal to the
Tom> >       number of columns in the result set. if you don't you'll core dump!
Tom> 
Tom> I really doubt that avoiding PQgetvalue() calls is worth taking those
Tom> kinds of risks and narrowing our options for future reimplementation of
Tom> the library and protocol.

i agree that it limits future choices. i'm just trying to port a ORCL/SQLServer
application to postgresql w/ the least possible change & w/o incurring
the copying overheaded currently in libpq.

Tom> 
Tom> In the long run we'll probably be moving to some more-modern interface
Tom> design like CORBA, which should help to address performance concerns
Tom> like these.

how would CORBA help?

thanks, jr

-- 
------------------------------------------------------------------------
Joel W. Reed                              http://ruby.ddiworld.com/jreed
----------------We're lost, but we're making good time.----------------

Joel Reed <jreed@support.ddiworld.com> writes:
Tom> In the long run we'll probably be moving to some more-modern interface
Tom> design like CORBA, which should help to address performance concerns
Tom> like these.

> how would CORBA help?

Bearing in mind that I know very little about CORBA ;-), I believe the
main way it'd help us in this particular area is that it provides
support for platform-independent transmission of binary datatypes.
For example, you send an integer, you get an integer; if byte-swapping
is needed it happens automatically and you don't have to think about it.
This'd be a big win performance-wise compared to Postgres' current
approach of converting everything to ASCII strings and then having to
convert back in client code.

Now I realize that you seemed to be concerned about programming
convenience as much as shaving cycles.  I don't know enough about CORBA
to say if it helps much in that regard; perhaps someone else can answer?

BTW, I do not intend to imply that moving Postgres to CORBA is a
foregone conclusion.  A couple of people have argued for it, but
not much has been done yet; the door is certainly still open to
other ideas.  I'm just trying to point out that a complete replacement
of libpq with some other API might be a better long-term answer than
nibbling away at the edges of the API model that libpq provides.
        regards, tom lane

On Mon, Feb 14, 2000 at 10:57:03AM -0500, Tom Lane wrote:
> > "libpq" like API's buffers to my memory locations) in my days working
> > with ODBC, Oracle & SQLServer's C-API, i created the following patch
> > which i thought i'd submit. I've included a README as well. 

Something like that is possible with ECPG. Just give use an arraypointer as
variable and set it to NULL. After fetching the date it contains the data,
the memory is allocated by libecpg. Not exactly what you are talking about
but similar.

Michael
-- 
Michael Meskes                         | Go SF 49ers!
Th.-Heuss-Str. 61, D-41812 Erkelenz    | Go Rhein Fire!
Tel.: (+49) 2431/72651                 | Use Debian GNU/Linux!
Email: Michael@Fam-Meskes.De           | Use PostgreSQL!