On Thu, 2021-06-17 at 12:42 -0400, Robert Haas wrote:
> On a casual read-through this seems pretty reasonable, but it
> essentially documents that libpq is doing the wrong thing by sending
> Sync unconditionally. As I say above, I disagree with that from a
> philosophical perspective. Then again, unless we're willing to
> redefine the wire protocol, I don't have an alternative to offer.
What if we simply mandate that a Sync must be sent before the server
will respond with CopyInResponse/CopyBothResponse, and the client must
send another Sync after CopyDone/CopyFail (or after receiving an
ErrorResponse, if the client isn't going to send a CopyDone/CopyFail)?
This will follow what libpq is already doing today, as far as I can
tell, and it will leave the server in a definite state.
In theory, it could break a client that issues Parse+Bind+Execute for a
CopyIn/CopyBoth command without a Sync, but I'm not sure there are any
clients that do that, and it's arguable whether the documentation
permitted that or not anyway.
I hacked together a quick patch; attached.
Regards,
Jeff Davis
diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index fdf57f15560..f936f76a257 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -171,16 +171,43 @@ ReceiveCopyBegin(CopyFromState cstate)
StringInfoData buf;
int natts = list_length(cstate->attnumlist);
int16 format = (cstate->opts.binary ? 1 : 0);
+ int mtype;
int i;
+ cstate->copy_src = COPY_FRONTEND;
+ cstate->fe_msgbuf = makeStringInfo();
+
+ HOLD_CANCEL_INTERRUPTS();
+ pq_startmsgread();
+ mtype = pq_getbyte();
+
+ if (mtype == EOF)
+ ereport(ERROR,
+ (errcode(ERRCODE_CONNECTION_FAILURE),
+ errmsg("unexpected EOF on client connection with an open transaction")));
+
+ /* expecting a Sync */
+ if (mtype != 'S')
+ ereport(ERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("unexpected message type 0x%02X before COPY FROM STDIN; expected Sync",
+ mtype)));
+
+ /* Now collect the message body */
+ if (pq_getmessage(cstate->fe_msgbuf, PQ_SMALL_MESSAGE_LIMIT))
+ ereport(ERROR,
+ (errcode(ERRCODE_CONNECTION_FAILURE),
+ errmsg("unexpected EOF on client connection with an open transaction")));
+
+ RESUME_CANCEL_INTERRUPTS();
+
pq_beginmessage(&buf, 'G');
pq_sendbyte(&buf, format); /* overall format */
pq_sendint16(&buf, natts);
for (i = 0; i < natts; i++)
pq_sendint16(&buf, format); /* per-column formats */
pq_endmessage(&buf);
- cstate->copy_src = COPY_FRONTEND;
- cstate->fe_msgbuf = makeStringInfo();
+
/* We *must* flush here to ensure FE knows it can send. */
pq_flush();
}
