Обсуждение: Adding a note to protocol.sgml regarding CopyData
In libpq.sgml following is stated:
Before <productname>PostgreSQL</productname> protocol 3.0, it was necessary
for the application to explicitly send the two characters
<literal>\.</literal> as a final line to indicate to the server that it had
finished sending <command>COPY</command> data. While this still works, it is deprecated and the
special meaning of <literal>\.</literal> can be expected to be removed in a
future release. It is sufficient to call <function>PQendcopy</function> after
having sent the actual data.
I think this should be mentioned in protocol.sgml as well. Developers
who wish to develop programs that understand frontend/backend protocol
should be able to focus on protocol.sgml. Attached is a patch for
this.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jpdiff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 46d7e19..a98e4af 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1154,6 +1154,20 @@ SELCT 1/0;
(if successful) or ErrorResponse (if not).
</para>
+ <note>
+ <para>
+ Before <productname>PostgreSQL</productname> protocol 3.0, it was necessary
+ for the application to explicitly send the two characters
+ <literal>\.</literal> as a final line to indicate to the server that it
+ had finished sending <command>COPY</command> data. Programs
+ implementing <command>COPY</command> in protocol 3.0
+ including <productname>PostgreSQL</productname> need to check and
+ ignore
+ <literal>\.</literal> just before COPYDone message for backward
+ compatibility sake. This requirement may be removed in the future.
+ </para>
+ </note>
+
<para>
In the event of a backend-detected error during copy-in mode (including
receipt of a CopyFail message), the backend will issue an ErrorResponse
Hello Tatsuo-san, Minor suggestions, although I'm not a native English speaker. > In libpq.sgml following is stated: > > Before <productname>PostgreSQL</productname> protocol 3.0, it was necessary > for the application to explicitly send the two characters > <literal>\.</literal> as a final line to indicate to the server that it ISTM That "it" may refer to the server... Put "the application" instead? > had > finished sending <command>COPY</command> data. While this still works, it is deprecated and the > special meaning of <literal>\.</literal> can be expected to be removed in a > future release. Maybe be more assertive, "will be removed"? > It is sufficient to call <function>PQendcopy</function> after "It is now sufficient ..."? > having sent the actual data. > > I think this should be mentioned in protocol.sgml as well. Developers > who wish to develop programs that understand frontend/backend protocol > should be able to focus on protocol.sgml. Attached is a patch for > this. -- Fabien.
Hi Fabien, Thank you for the comment. > Hello Tatsuo-san, > > Minor suggestions, although I'm not a native English speaker. > >> In libpq.sgml following is stated: >> >> Before <productname>PostgreSQL</productname> protocol 3.0, it was >> necessary >> for the application to explicitly send the two characters >> <literal>\.</literal> as a final line to indicate to the server that >> it > > ISTM That "it" may refer to the server... Put "the application" > instead? > >> had >> finished sending <command>COPY</command> data. While this still >> works, it is deprecated and the >> special meaning of <literal>\.</literal> can be expected to be removed >> in a >> future release. > > Maybe be more assertive, "will be removed"? > >> It is sufficient to call <function>PQendcopy</function> after > > "It is now sufficient ..."? Well, I did not intend to enhance libpq.sgml but maybe your points is valid (I cannot judge because I am not an native English speaker). So I am include your point to the patch and wait for feed back from (possibly) native English speakers. I also added to September commit fest, including you as a reviewer. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index d67212b..844d4a9 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -5754,11 +5754,12 @@ int PQputline(PGconn *conn, <para> Before <productname>PostgreSQL</productname> protocol 3.0, it was necessary for the application to explicitly send the two characters - <literal>\.</literal> as a final line to indicate to the server that it had - finished sending <command>COPY</command> data. While this still works, it is deprecated and the - special meaning of <literal>\.</literal> can be expected to be removed in a - future release. It is sufficient to call <function>PQendcopy</function> after - having sent the actual data. + <literal>\.</literal> as a final line to indicate to the server that + the application had finished sending <command>COPY</command> data. + While this still works, it is deprecated and the special meaning + of <literal>\.</literal> will be removed in a future release. It is + sufficient to call <function>PQendcopy</function> after having sent + the actual data. </para> </note> </listitem> diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 46d7e19..fda989b 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -1154,6 +1154,20 @@ SELCT 1/0; (if successful) or ErrorResponse (if not). </para> + <note> + <para> + Before <productname>PostgreSQL</productname> protocol 3.0, it was necessary + for the application to explicitly send the two characters + <literal>\.</literal> as a final line to indicate to the server that + the application had finished sending <command>COPY</command> data. + Programs implementing <command>COPY</command> in protocol 3.0 + including <productname>PostgreSQL</productname> need to check and + ignore + <literal>\.</literal> just before COPYDone message for backward + compatibility sake. This requirement will be removed in the future. + </para> + </note> + <para> In the event of a backend-detected error during copy-in mode (including receipt of a CopyFail message), the backend will issue an ErrorResponse
Hello Tatsuo-san, >> Minor suggestions, although I'm not a native English speaker. > Well, I did not intend to enhance libpq.sgml but maybe your points is > valid (I cannot judge because I am not an native English speaker). Argh, sorry, I did not read the right part:-( The note looks good to me. Doc build is ok. > So I am include your point to the patch and wait for feed back from > (possibly) native English speakers. Indeed. As Tom said in another thread, a good part of the documentation has been written by non native English speaker, and it shows. > I also added to September commit fest, including you as a reviewer. Hmmm... Already having a committer may deter other people to review it, while it is exactly what is desired. -- Fabien.
On 2018-08-25, Tatsuo Ishii wrote to the pgsql-docs mailing list ... > Hi Bradley, > Thank you for your follow up. Your patch looks good to me. > Can you please re-send your message in pgsql-hackers attaching to this thread ... > CommitFest app does not allow ... emails other than posted to pgsql-hackers. So I > decided to post to pgsql-hackers after posting to pgsql-docs. ... OK, I think this is what you wanted. Fabian's suggestion on making the removal more assertive is included in the v2 patch. On 2018-08- Bradley DeJong wrote ... >On 2018-07-27, Tatsuo Ishii wrote ... >>... I think this should be mentioned in protocol.sgml as well. ... > > I agree. It is already mentioned as one of the differences between v2 > and v3 but an implementer should not need to read that section if they > are only implementing v3. (I know I've never looked at it before.) > > Using protocol.diff as a base, I changed the phrasing to be more > prescriptive for v3 protocol implementers (don't send a final line, be > prepared to receive a final line), changed passive voice to active > voice and fixed one COPYData -> CopyData capitalization. > > I also called this out in the description of the CopyData message > format because that is where the termination line would be > transmitted.
Вложения
> On 2018-08-25, Tatsuo Ishii wrote to the pgsql-docs mailing list ... >> Hi Bradley, >> Thank you for your follow up. Your patch looks good to me. >> Can you please re-send your message in pgsql-hackers attaching to this >> thread ... >> CommitFest app does not allow ... emails other than posted to >> pgsql-hackers. So I >> decided to post to pgsql-hackers after posting to pgsql-docs. ... > > OK, I think this is what you wanted. Perfect. BTW I would like to add you to the co-author for the patch. It seems CF app requires you to have PostgreSQL community account. Do you have one? > Fabian's suggestion on making the removal more assertive is included > in the v2 patch. Looks good to me. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Hello Bradley & Tatsuo-san, My 0.02€ on the text: > Version 2.0 of the <productname>PostgreSQL</productname> protocol > > In the v3.0 protocol, > > the 3.0 protocol > > version 3.0 of the copy-in/copy-out sub-protocol > > the V2.0 protocol. While reading nice English (I learned "holdover"), it occurs to me that references to the protocol version lacks homogeneity. Should it use v, V or version? I'd suggest to keep "the vX.0 protocol" for a short version, and "the version X.0 protocol" for long if really desired. -- Fabien.
> Hello Bradley & Tatsuo-san, > > My 0.02€ on the text: > >> Version 2.0 of the <productname>PostgreSQL</productname> protocol >> In the v3.0 protocol, >> the 3.0 protocol >> version 3.0 of the copy-in/copy-out sub-protocol >> the V2.0 protocol. > > While reading nice English (I learned "holdover"), it occurs to me > that references to the protocol version lacks homogeneity. > > Should it use v, V or version? > > I'd suggest to keep "the vX.0 protocol" for a short version, and "the > version X.0 protocol" for long if really desired. +1 Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
On 2018-08-27, Fabien COELHO wrote ... > ... references to the protocol version lacks homogeneity. > ... I'd suggest to keep "the vX.0 protocol" for a short version, > and "the version X.0 protocol" for long ... I agree. Change made.
Вложения
Hello Bradley & Tatsuo-san, >> ... references to the protocol version lacks homogeneity. >> ... I'd suggest to keep "the vX.0 protocol" for a short version, >> and "the version X.0 protocol" for long ... > > I agree. Change made. Patch applies cleanly. Doc build ok. One part talks about "terminating line", the other of "termination line". I wonder whether one is better than the other? -- Fabien.
> Hello Bradley & Tatsuo-san, > >>> ... references to the protocol version lacks homogeneity. >>> ... I'd suggest to keep "the vX.0 protocol" for a short version, >>> and "the version X.0 protocol" for long ... >> >> I agree. Change made. > > Patch applies cleanly. Doc build ok. > > One part talks about "terminating line", the other of "termination > line". I wonder whether one is better than the other? I am not a native English speaker so I maybe wrong... for me, current usage of "terminating line", and "termination line" looks correct. The former refers to concrete example thus uses "the", while the latter refers to more general case thus uses "an". BTW, I think the patch should apply to master and REL_11_STABLE branches at least. But should this be applied to other back branches as well? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
>> Hello Bradley & Tatsuo-san, >> >>>> ... references to the protocol version lacks homogeneity. >>>> ... I'd suggest to keep "the vX.0 protocol" for a short version, >>>> and "the version X.0 protocol" for long ... >>> >>> I agree. Change made. >> >> Patch applies cleanly. Doc build ok. >> >> One part talks about "terminating line", the other of "termination >> line". I wonder whether one is better than the other? > > I am not a native English speaker so I maybe wrong... for me, current > usage of "terminating line", and "termination line" looks correct. The > former refers to concrete example thus uses "the", while the latter > refers to more general case thus uses "an". > > BTW, I think the patch should apply to master and REL_11_STABLE > branches at least. But should this be applied to other back branches > as well? I have marked this as "ready for committer". Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
On Mon, Sep 10, 2018 at 3:54 PM Tatsuo Ishii <ishii@sraoss.co.jp> wrote: > > >> Hello Bradley & Tatsuo-san, > >> > >>>> ... references to the protocol version lacks homogeneity. > >>>> ... I'd suggest to keep "the vX.0 protocol" for a short version, > >>>> and "the version X.0 protocol" for long ... > >>> > >>> I agree. Change made. > >> > >> Patch applies cleanly. Doc build ok. > >> > >> One part talks about "terminating line", the other of "termination > >> line". I wonder whether one is better than the other? > > > > I am not a native English speaker so I maybe wrong... for me, current > > usage of "terminating line", and "termination line" looks correct. The > > former refers to concrete example thus uses "the", while the latter > > refers to more general case thus uses "an". > > > > BTW, I think the patch should apply to master and REL_11_STABLE > > branches at least. But should this be applied to other back branches > > as well? > > I have marked this as "ready for committer". > My first thought on this patch is that why do we want to duplicate the same information in different words at three different places. Why can't we just extend the current Note where it is currently with some more information about CopyDone message and then add a reference to that section in other two places? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Thanks for the feedback.
On 2018-09-22, Amit Kapila wrote ...
> ... Why can't we just extend the current Note where it is currently
...
Because information about how the protocol works belongs in the protocol
documentation not in the documentation for one implementation of the
protocol.
Think of it this way, if the only full explanation of this information
was in the psqlODBC or pgJDBC documentation would you feel comfortable
just referencing it from protocol.sgml? I would not and, in my opinion,
libpq's being the reference client implementation should not change
that.
On top of that, in the libpq documentation the termination line is only
mentioned in a section titled "Obsolete Functions for COPY" which makes
it even less likely that someone working on a different implementation
of the protocol will notice it.
[strong opinion - I would object to leaving the only description in the
libpq documentation]
> ... why do we want to duplicate the same information ...
The change to the CopyData message documentation does refer back to the
full description. My intent with the brief description of the \. line
was to include enough information so that the reader could decide
whether or not skipping back to the full description would be useful. I
think that usefulness outweighs the minor duplication.
[moderate opinion - I plan to leave it as is unless others weigh in in
favor of just keeping the reference]
But given that I don't work on libpq or even use it, I'm not comfortable
changing the documentation of 4 different libpq methods (even obsolete
methods) on my own initiative. If the committer who picks this up wants
the libpq documentation changed as part of this, that would be different
and I'd be willing to give it a shot.
[no strong feelings one way or the other - I would leave the libpq
documentation as is but could easily be swayed]
> ... duplicate the same information in different words at three
different places ...
I count 7 different places. In the protocol docs, there is the old
mention in the "Summary of Changes since Protocol 2.0" section and the
two new mentions in the protocol definitions plus after reading through
libpq-copy.html again, I think all 4 of these sections refer to the
terminating line/end-of-copy-data marker.
PQgetline - "... the application must check to see if a new line
consists of the two characters \., which indicates ..."
PQgetlineAsync - "... returns -1 if the end-of-copy-data marker has
been recognized ..."
PQputline - "... Note ...send the two characters \. as a final line
..."
PQendcopy - "... followed by PQendcopy after the terminator line is
seen ..."
[informational - lists references to remove when a future protocol drops
the \. line entirely]
On Tue, Sep 25, 2018 at 4:51 AM Bradley DeJong <bpd0018@gmail.com> wrote: > > On 2018-09-22, Amit Kapila wrote ... > > ... duplicate the same information in different words at three > different places ... > > I count 7 different places. In the protocol docs, there is the old > mention in the "Summary of Changes since Protocol 2.0" section > Below text is present in the section quoted by you: The special <quote><literal>\.</literal></quote> last line is not needed anymore, and is not sent during <command>COPY OUT</command>. (It is still recognized as a terminator during <command>COPY IN</command>, but its use is deprecated and will eventually be removed.) This email started with the need to mention this in protocol.sgml and it is already present although at a different place than the place at which it is proposed. Personally, I don't see the need to add it to more places. Does anybody else have any opinion on this matter? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On 25/09/2018 13:55, Amit Kapila wrote: > On Tue, Sep 25, 2018 at 4:51 AM Bradley DeJong <bpd0018@gmail.com> wrote: >> >> On 2018-09-22, Amit Kapila wrote ... >> > ... duplicate the same information in different words at three >> different places ... >> >> I count 7 different places. In the protocol docs, there is the old >> mention in the "Summary of Changes since Protocol 2.0" section >> > > Below text is present in the section quoted by you: > The special <quote><literal>\.</literal></quote> last line is not > needed anymore, and is not sent during <command>COPY OUT</command>. > (It is still recognized as a terminator during <command>COPY > IN</command>, but its use is deprecated and will eventually be > removed.) > > This email started with the need to mention this in protocol.sgml and > it is already present although at a different place than the place at > which it is proposed. Personally, I don't see the need to add it to > more places. Does anybody else have any opinion on this matter? Yeah, I don't see why we need to document it three times in the same chapter. Also, that chapter is specifically about version 3.0 of the protocol, so documenting version 2.0 is out of scope. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Sep 28, 2018 at 08:16:46PM +0200, Peter Eisentraut wrote: > Yeah, I don't see why we need to document it three times in the same > chapter. > > Also, that chapter is specifically about version 3.0 of the protocol, so > documenting version 2.0 is out of scope. This has been marked as returned with feedback per the last bits of the thread. -- Michael