On Wed, Apr 04, 2012 at 06:41:00PM -0400, Tom Lane wrote:
> Marko Kreen <mark...@gmail.com> writes:
> > On Wed, Apr 4, 2012 at 10:17 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> >> Given the lack of consensus around the suspension API, maybe the best
> >> way to get the underlying libpq patch to a committable state is to take
> >> it out --- that is, remove the "return zero" option for row processors.
>
> > Agreed.
>
> Done that way.
Minor cleanups:
* Change callback return value to be 'bool': 0 is error.
Currently the accepted return codes are 1 and -1,
which is weird.
If we happen to have the 'early-exit' logic in the future,
it should not work via callback return code. So keeping the 0
in reserve is unnecessary.
* Support exceptions in multi-statement PQexec() by storing
finished result under PGconn temporarily. Without doing it,
the result can leak if callback longjmps while processing
next result.
* Add <caution> to docs for permanently keeping custom callback.
This API fragility is also reason why early-exit (if it appears)
should not work via callback - instead it should give safer API.
--
marko
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0ec501e..1e7678c 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -5608,6 +5608,16 @@ defaultNoticeProcessor(void *arg, const char *message)
<caution>
<para>
+ It's dangerous to leave custom row processor attached to connection
+ permanently as there might be occasional queries that expect
+ regular PGresult behaviour. So unless it is certain all code
+ is familiar with custom callback, it is better to restore default
+ row processor after query is finished.
+ </para>
+ </caution>
+
+ <caution>
+ <para>
The row processor function sees the rows before it is known whether the
query will succeed overall, since the server might return some rows before
encountering an error. For proper transactional behavior, it must be
@@ -5674,8 +5684,8 @@ typedef struct pgDataValue
<parameter>errmsgp</parameter> is an output parameter used only for error
reporting. If the row processor needs to report an error, it can set
<literal>*</><parameter>errmsgp</parameter> to point to a suitable message
- string (and then return <literal>-1</>). As a special case, returning
- <literal>-1</> without changing <literal>*</><parameter>errmsgp</parameter>
+ string (and then return <literal>0</>). As a special case, returning
+ <literal>0</> without changing <literal>*</><parameter>errmsgp</parameter>
from its initial value of NULL is taken to mean <quote>out of memory</>.
</para>
@@ -5702,10 +5712,10 @@ typedef struct pgDataValue
<para>
The row processor function must return either <literal>1</> or
- <literal>-1</>.
+ <literal>0</>.
<literal>1</> is the normal, successful result value; <application>libpq</>
will continue with receiving row values from the server and passing them to
- the row processor. <literal>-1</> indicates that the row processor has
+ the row processor. <literal>0</> indicates that the row processor has
encountered an error. In that case,
<application>libpq</> will discard all remaining rows in the result set
and then return a <literal>PGRES_FATAL_ERROR</> <type>PGresult</type> to
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 03fd6e4..90f6d6a 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -2897,6 +2897,11 @@ closePGconn(PGconn *conn)
* absent */
conn->asyncStatus = PGASYNC_IDLE;
pqClearAsyncResult(conn); /* deallocate result */
+ if (conn->tempResult)
+ {
+ PQclear(conn->tempResult);
+ conn->tempResult = NULL;
+ }
pg_freeaddrinfo_all(conn->addrlist_family, conn->addrlist);
conn->addrlist = NULL;
conn->addr_cur = NULL;
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 86f157c..554df94 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1028,7 +1028,7 @@ PQgetRowProcessor(const PGconn *conn, void **param)
/*
* pqStdRowProcessor
* Add the received row to the PGresult structure
- * Returns 1 if OK, -1 if error occurred.
+ * Returns 1 if OK, 0 if error occurred.
*
* Note: "param" should point to the PGconn, but we don't actually need that
* as of the current coding.
@@ -1059,7 +1059,7 @@ pqStdRowProcessor(PGresult *res, const PGdataValue *columns,
tup = (PGresAttValue *)
pqResultAlloc(res, nfields * sizeof(PGresAttValue), TRUE);
if (tup == NULL)
- return -1;
+ return 0;
for (i = 0; i < nfields; i++)
{
@@ -1078,7 +1078,7 @@ pqStdRowProcessor(PGresult *res, const PGdataValue *columns,
val = (char *) pqResultAlloc(res, clen + 1, isbinary);
if (val == NULL)
- return -1;
+ return 0;
/* copy and zero-terminate the data (even if it's binary) */
memcpy(val, columns[i].value, clen);
@@ -1091,7 +1091,7 @@ pqStdRowProcessor(PGresult *res, const PGdataValue *columns,
/* And add the tuple to the PGresult's tuple array */
if (!pqAddTuple(res, tup))
- return -1;
+ return 0;
/* Success */
return 1;
@@ -1954,6 +1954,13 @@ PQexecFinish(PGconn *conn)
PGresult *result;
PGresult *lastResult;
+ /* Make sure old result is NULL. */
+ if (conn->tempResult)
+ {
+ PQclear(conn->tempResult);
+ conn->tempResult = NULL;
+ }
+
/*
* For backwards compatibility, return the last result if there are more
* than one --- but merge error messages if we get more than one error
@@ -1991,7 +1998,14 @@ PQexecFinish(PGconn *conn)
result->resultStatus == PGRES_COPY_BOTH ||
conn->status == CONNECTION_BAD)
break;
+
+ /*
+ * As we know previous result is stored from this function,
+ * there is no need for extra cleanup here.
+ */
+ conn->tempResult = lastResult;
}
+ conn->tempResult = NULL;
return lastResult;
}
diff --git a/src/interfaces/libpq/fe-protocol2.c b/src/interfaces/libpq/fe-protocol2.c
index 43f9954..f155678 100644
--- a/src/interfaces/libpq/fe-protocol2.c
+++ b/src/interfaces/libpq/fe-protocol2.c
@@ -761,7 +761,7 @@ getRowDescriptions(PGconn *conn)
/* everything is good */
return 0;
- case -1:
+ case 0:
/* error, report the errmsg below */
break;
@@ -950,7 +950,7 @@ getAnotherTuple(PGconn *conn, bool binary)
/* everything is good */
return 0;
- case -1:
+ case 0:
/* error, report the errmsg below */
break;
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index a773d7a..9124a87 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -589,7 +589,7 @@ getRowDescriptions(PGconn *conn, int msgLength)
/* everything is good */
return 0;
- case -1:
+ case 0:
/* error, report the errmsg below */
break;
@@ -793,7 +793,7 @@ getAnotherTuple(PGconn *conn, int msgLength)
/* everything is good */
return 0;
- case -1:
+ case 0:
/* error, report the errmsg below */
break;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 0b6e676..7f0f8fd 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -404,6 +404,9 @@ struct pg_conn
PGdataValue *rowBuf; /* array for passing values to rowProcessor */
int rowBufLen; /* number of entries allocated in rowBuf */
+ /* support rowproc exceptions in multi-resultset functions (PQexec) */
+ PGresult *tempResult; /* temp result storage */
+
/* Status for asynchronous result construction */
PGresult *result; /* result being constructed */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
