I wrote:
> Or we could rethink the logic. It's not quite clear to me, after
> all this time, why getRowDescriptions() et al are allowed to
> move inStart themselves rather than letting the main loop in
> pqParseInput3 do it. It might well be an artifact of having not
> rewritten the v2 logic too much.
After studying this further, I think we should apply the attached
patch to remove that responsibility from pqParseInput3's subroutines.
This will allow a single trace call near the bottom of pqParseInput3
to handle all cases that that function processes.
There are still some cowboy advancements of inStart in the functions
concerned with COPY processing, but it doesn't look like there's
much to be done about that. Those spots will need their own trace
calls.
BTW, while looking at this I concluded that getParamDescriptions
is actually buggy: if it's given a malformed message that seems
to need more data than the message length specifies, it just
returns EOF, resulting in an infinite loop. This function apparently
got missed while converting the logic from v2 (where that was the
right thing) to v3 (where it ain't). So that bit needs to be
back-patched. I'm tempted to back-patch the whole thing though.
regards, tom lane
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 2ca8c057b9..233456fd90 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -290,8 +290,6 @@ pqParseInput3(PGconn *conn)
/* First 'T' in a query sequence */
if (getRowDescriptions(conn, msgLength))
return;
- /* getRowDescriptions() moves inStart itself */
- continue;
}
else
{
@@ -337,8 +335,7 @@ pqParseInput3(PGconn *conn)
case 't': /* Parameter Description */
if (getParamDescriptions(conn, msgLength))
return;
- /* getParamDescriptions() moves inStart itself */
- continue;
+ break;
case 'D': /* Data Row */
if (conn->result != NULL &&
conn->result->resultStatus == PGRES_TUPLES_OK)
@@ -346,8 +343,6 @@ pqParseInput3(PGconn *conn)
/* Read another tuple of a normal query response */
if (getAnotherTuple(conn, msgLength))
return;
- /* getAnotherTuple() moves inStart itself */
- continue;
}
else if (conn->result != NULL &&
conn->result->resultStatus == PGRES_FATAL_ERROR)
@@ -462,7 +457,6 @@ handleSyncLoss(PGconn *conn, char id, int msgLength)
* command for a prepared statement) containing the attribute data.
* Returns: 0 if processed message successfully, EOF to suspend parsing
* (the latter case is not actually used currently).
- * In the former case, conn->inStart has been advanced past the message.
*/
static int
getRowDescriptions(PGconn *conn, int msgLength)
@@ -577,9 +571,6 @@ getRowDescriptions(PGconn *conn, int msgLength)
/* Success! */
conn->result = result;
- /* Advance inStart to show that the "T" message has been processed. */
- conn->inStart = conn->inCursor;
-
/*
* If we're doing a Describe, we're done, and ready to pass the result
* back to the client.
@@ -603,9 +594,6 @@ advance_and_error:
if (result && result != conn->result)
PQclear(result);
- /* Discard the failed message by pretending we read it */
- conn->inStart += 5 + msgLength;
-
/*
* Replace partially constructed result with an error result. First
* discard the old result to try to win back some memory.
@@ -624,6 +612,12 @@ advance_and_error:
appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
pqSaveErrorResult(conn);
+ /*
+ * Show the message as fully consumed, else pqParseInput3 will overwrite
+ * our error with a complaint about that.
+ */
+ conn->inCursor = conn->inStart + 5 + msgLength;
+
/*
* Return zero to allow input parsing to continue. Subsequent "D"
* messages will be ignored until we get to end of data, since an error
@@ -635,12 +629,8 @@ advance_and_error:
/*
* parseInput subroutine to read a 't' (ParameterDescription) message.
* We'll build a new PGresult structure containing the parameter data.
- * Returns: 0 if completed message, EOF if not enough data yet.
- * In the former case, conn->inStart has been advanced past the message.
- *
- * Note that if we run out of data, we have to release the partially
- * constructed PGresult, and rebuild it again next time. Fortunately,
- * that shouldn't happen often, since 't' messages usually fit in a packet.
+ * Returns: 0 if processed message successfully, EOF to suspend parsing
+ * (the latter case is not actually used currently).
*/
static int
getParamDescriptions(PGconn *conn, int msgLength)
@@ -690,23 +680,16 @@ getParamDescriptions(PGconn *conn, int msgLength)
/* Success! */
conn->result = result;
- /* Advance inStart to show that the "t" message has been processed. */
- conn->inStart = conn->inCursor;
-
return 0;
not_enough_data:
- PQclear(result);
- return EOF;
+ errmsg = libpq_gettext("insufficient data in \"t\" message");
advance_and_error:
/* Discard unsaved result, if any */
if (result && result != conn->result)
PQclear(result);
- /* Discard the failed message by pretending we read it */
- conn->inStart += 5 + msgLength;
-
/*
* Replace partially constructed result with an error result. First
* discard the old result to try to win back some memory.
@@ -724,6 +707,12 @@ advance_and_error:
appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
pqSaveErrorResult(conn);
+ /*
+ * Show the message as fully consumed, else pqParseInput3 will overwrite
+ * our error with a complaint about that.
+ */
+ conn->inCursor = conn->inStart + 5 + msgLength;
+
/*
* Return zero to allow input parsing to continue. Essentially, we've
* replaced the COMMAND_OK result with an error result, but since this
@@ -737,7 +726,6 @@ advance_and_error:
* We fill rowbuf with column pointers and then call the row processor.
* Returns: 0 if processed message successfully, EOF to suspend parsing
* (the latter case is not actually used currently).
- * In the former case, conn->inStart has been advanced past the message.
*/
static int
getAnotherTuple(PGconn *conn, int msgLength)
@@ -817,21 +805,14 @@ getAnotherTuple(PGconn *conn, int msgLength)
goto advance_and_error;
}
- /* Advance inStart to show that the "D" message has been processed. */
- conn->inStart = conn->inCursor;
-
/* Process the collected row */
errmsg = NULL;
if (pqRowProcessor(conn, &errmsg))
return 0; /* normal, successful exit */
- goto set_error_result; /* pqRowProcessor failed, report it */
+ /* pqRowProcessor failed, fall through to report it */
advance_and_error:
- /* Discard the failed message by pretending we read it */
- conn->inStart += 5 + msgLength;
-
-set_error_result:
/*
* Replace partially constructed result with an error result. First
@@ -851,6 +832,12 @@ set_error_result:
appendPQExpBuffer(&conn->errorMessage, "%s\n", errmsg);
pqSaveErrorResult(conn);
+ /*
+ * Show the message as fully consumed, else pqParseInput3 will overwrite
+ * our error with a complaint about that.
+ */
+ conn->inCursor = conn->inStart + 5 + msgLength;
+
/*
* Return zero to allow input parsing to continue. Subsequent "D"
* messages will be ignored until we get to end of data, since an error
