At Mon, 6 Mar 2017 11:13:48 +0100, Petr Jelinek <petr.jeli...@2ndquadrant.com>
wrote in <a7c0cc59-9c21-238c-b6a7-d6eb519ad...@2ndquadrant.com>
> >>> Well the length is necessary to be able to add binary format support in
> >>> future so it's definitely not an omission.
> >>
> >> Right. So it appears the right function to use here is
> >> pq_sendcountedtext(). However, this sends the strings without null
> >> termination, so we'd have to do extra processing on the receiving end.
> >> Or we could add an option to pq_sendcountedtext() to make it do what we
> >> want. I'd rather stick to standard pqformat.c functions for handling
> >> the protocol.
> >
> > It seems reasonable. I changed the prototype of
> > pg_sendcountedtext as the following,
> >
> > | extern void pq_sendcountedtext(StringInfo buf, const char *str, int slen,
> > | bool countincludesself, bool binary);
> >
> > I think 'binary' seems fine for the parameter here. The patch
> > size increased a bit.
> >
>
> Hmm I find it bit weird that binary true means NULL terminated while
> false means not NULL terminated, I would think that the behaviour would
> be exactly opposite given that text strings are the ones where NULL
> termination matters?
You're right. I renamed it as with more straightforward
name. Addition to that, it contained a stupid bug that sends a
garbage byte when non-null-terminated string is not converted.
And the comment is edited to reflect the new behavior.
> > By the way, I noticed that postmaster launches logical
> > replication launcher even if wal_level < logical. Is it right?
>
> Yes, that came up couple of times in various threads. The logical
> replication launcher is needed only to start subscriptions and
> subscriptions don't need wal_level to be logical, only publication needs
> that.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/access/common/printsimple.c b/src/backend/access/common/printsimple.c
index 5fe1c72..62fa70d 100644
--- a/src/backend/access/common/printsimple.c
+++ b/src/backend/access/common/printsimple.c
@@ -96,7 +96,7 @@ printsimple(TupleTableSlot *slot, DestReceiver *self)
pq_sendcountedtext(&buf,
VARDATA_ANY(t),
VARSIZE_ANY_EXHDR(t),
- false);
+ false, false);
}
break;
@@ -106,7 +106,7 @@ printsimple(TupleTableSlot *slot, DestReceiver *self)
char str[12]; /* sign, 10 digits and '\0' */
pg_ltoa(num, str);
- pq_sendcountedtext(&buf, str, strlen(str), false);
+ pq_sendcountedtext(&buf, str, strlen(str), false, false);
}
break;
@@ -116,7 +116,7 @@ printsimple(TupleTableSlot *slot, DestReceiver *self)
char str[23]; /* sign, 21 digits and '\0' */
pg_lltoa(num, str);
- pq_sendcountedtext(&buf, str, strlen(str), false);
+ pq_sendcountedtext(&buf, str, strlen(str), false, false);
}
break;
diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c
index a2ca2d7..1ebc50f 100644
--- a/src/backend/access/common/printtup.c
+++ b/src/backend/access/common/printtup.c
@@ -353,7 +353,8 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
char *outputstr;
outputstr = OutputFunctionCall(&thisState->finfo, attr);
- pq_sendcountedtext(&buf, outputstr, strlen(outputstr), false);
+ pq_sendcountedtext(&buf, outputstr, strlen(outputstr),
+ false, false);
}
else
{
@@ -442,7 +443,7 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
Assert(thisState->format == 0);
outputstr = OutputFunctionCall(&thisState->finfo, attr);
- pq_sendcountedtext(&buf, outputstr, strlen(outputstr), true);
+ pq_sendcountedtext(&buf, outputstr, strlen(outputstr), true, false);
}
pq_endmessage(&buf);
diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c
index c8cf67c..a83c73e 100644
--- a/src/backend/libpq/pqformat.c
+++ b/src/backend/libpq/pqformat.c
@@ -123,30 +123,39 @@ pq_sendbytes(StringInfo buf, const char *data, int datalen)
* The data sent to the frontend by this routine is a 4-byte count field
* followed by the string. The count includes itself or not, as per the
* countincludesself flag (pre-3.0 protocol requires it to include itself).
- * The passed text string need not be null-terminated, and the data sent
- * to the frontend isn't either.
+ * The passed text string need not be null-terminated but should not contain
+ * null as a part. sendterminator instructs to send null-terminator.
* --------------------------------
*/
void
pq_sendcountedtext(StringInfo buf, const char *str, int slen,
- bool countincludesself)
+ bool countincludesself, bool sendterminator)
{
- int extra = countincludesself ? 4 : 0;
+ int extra_self = countincludesself ? 4 : 0;
+ int extra_term = sendterminator ? 1 : 0;
char *p;
p = pg_server_to_client(str, slen);
if (p != str) /* actual conversion has been done? */
{
slen = strlen(p);
- pq_sendint(buf, slen + extra, 4);
+ pq_sendint(buf, slen + extra_self + extra_term, 4);
appendBinaryStringInfo(buf, p, slen);
pfree(p);
}
else
{
- pq_sendint(buf, slen + extra, 4);
+ pq_sendint(buf, slen + extra_self + extra_term, 4);
appendBinaryStringInfo(buf, str, slen);
+
}
+
+ /*
+ * str may not be null-terminated. Send the terminator separately from the
+ * given string.
+ */
+ if (sendterminator)
+ appendBinaryStringInfo(buf, "", 1);
}
/* --------------------------------
diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c
index bc6e9b5..c5e4214 100644
--- a/src/backend/replication/logical/proto.c
+++ b/src/backend/replication/logical/proto.c
@@ -417,7 +417,6 @@ logicalrep_write_tuple(StringInfo out, Relation rel, HeapTuple tuple)
Form_pg_type typclass;
Form_pg_attribute att = desc->attrs[i];
char *outputstr;
- int len;
/* skip dropped columns */
if (att->attisdropped)
@@ -442,9 +441,12 @@ logicalrep_write_tuple(StringInfo out, Relation rel, HeapTuple tuple)
pq_sendbyte(out, 't'); /* 'text' data follows */
outputstr = OidOutputFunctionCall(typclass->typoutput, values[i]);
- len = strlen(outputstr) + 1; /* null terminated */
- pq_sendint(out, len, 4); /* length */
- pq_sendstring(out, outputstr); /* data */
+
+ /*
+ * Here, the string is sent as binary data so the length field counts
+ * terminating NULL.
+ */
+ pq_sendcountedtext(out, outputstr, strlen(outputstr), false, true);
pfree(outputstr);
diff --git a/src/backend/tcop/fastpath.c b/src/backend/tcop/fastpath.c
index 2efed95..0ca9522 100644
--- a/src/backend/tcop/fastpath.c
+++ b/src/backend/tcop/fastpath.c
@@ -160,7 +160,8 @@ SendFunctionResult(Datum retval, bool isnull, Oid rettype, int16 format)
getTypeOutputInfo(rettype, &typoutput, &typisvarlena);
outputstr = OidOutputFunctionCall(typoutput, retval);
- pq_sendcountedtext(&buf, outputstr, strlen(outputstr), false);
+ pq_sendcountedtext(&buf, outputstr, strlen(outputstr),
+ false, false);
pfree(outputstr);
}
else if (format == 1)
diff --git a/src/include/libpq/pqformat.h b/src/include/libpq/pqformat.h
index 4df87ec..e724b04 100644
--- a/src/include/libpq/pqformat.h
+++ b/src/include/libpq/pqformat.h
@@ -19,7 +19,7 @@ extern void pq_beginmessage(StringInfo buf, char msgtype);
extern void pq_sendbyte(StringInfo buf, int byt);
extern void pq_sendbytes(StringInfo buf, const char *data, int datalen);
extern void pq_sendcountedtext(StringInfo buf, const char *str, int slen,
- bool countincludesself);
+ bool countincludesself, bool sendterminator);
extern void pq_sendtext(StringInfo buf, const char *str, int slen);
extern void pq_sendstring(StringInfo buf, const char *str);
extern void pq_send_ascii_string(StringInfo buf, const char *str);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
