Patch to document base64 encoding

Hello Karl,

Doc patch, against master. Documents encode() and decode() base64
format.

It is already documented. Enhance documentation, though.

Builds for me.

For me as well. Looks ok.

Attached: doc_base64_v1.patch

References RFC2045 section 6.8 to define base64.

Did you consider referencing RFC 4648 instead?

--
Fabien.

Hi Fabien,

On Tue, 5 Mar 2019 07:09:01 +0100 (CET)
Fabien COELHO <coelho@cri.ensmp.fr> wrote:

Doc patch, against master. Documents encode() and decode() base64
format.

It is already documented. Enhance documentation, though.

Right. I was thinking that there are various implementations
of the base64 data format and so it needed more than
just to be named.

Attached: doc_base64_v1.patch

References RFC2045 section 6.8 to define base64.

Did you consider referencing RFC 4648 instead?

Not really. What drew me to document was the line
breaks every 76 characters. So I pretty much went
straight to the MIME RFC which says there should
be breaks at 76 characters.

I can see advantages and disadvantages either way.
More or less extraneous information either semi
or not base64 related in either RFC.
Which RFC do you think should be referenced?

Attached: doc_base64_v2.patch

This new version adds a phrase clarifying that
decode errors are raised when trailing padding
is wrong. Seemed like I may as well be explicit.

(I am not entirely pleased with the double dash
but can't come up with anything better. And
can't make an emdash entity work either.)

Thanks for taking a look.

Regards,

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

On Tue, 5 Mar 2019 07:26:17 -0600
"Karl O. Pinc" <kop@meme.com> wrote:

(I am not entirely pleased with the double dash
but can't come up with anything better. And
can't make an emdash entity work either.)

Attached: doc_base64_v3.patch

There is an mdash entity. This patch uses that.

Regards,

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

Hello Karl,

Attached: doc_base64_v3.patch

I'm ok with referencing the historical MIME RFC.

"RFC2045 section 6.8" -> "RFC 2045 Section 6.8"

you can link to the RFC directly with:

<ulink url="https://tools.ietf.org/html/rfc2045#section-6.8&quot;&gt;RFC 2045
Section 6.8</ulink>

--
Fabien.

Hi Fabien,

On Tue, 5 Mar 2019 23:02:26 +0100 (CET)
Fabien COELHO <coelho@cri.ensmp.fr> wrote:

Attached: doc_base64_v3.patch

I'm ok with referencing the historical MIME RFC.

For the record, RFC 2045 is updated but not
yet obsolete. The updates don't invalidate
section 6.8.

"RFC2045 section 6.8" -> "RFC 2045 Section 6.8"

you can link to the RFC directly with:

<ulink url="https://tools.ietf.org/html/rfc2045#section-6.8&quot;&gt;RFC 2045
Section 6.8</ulink>

Done.

Attached: doc_base64_v4.patch

Regards,

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

On Tue, Mar 05, 2019 at 07:55:22PM -0600, Karl O. Pinc wrote:

Attached: doc_base64_v4.patch

Details about the "escape" mode are already available within the
description of function "encode". Wouldn't we want to consolidate a
description for all the modes at the same place, including some words
for hex? Your patch only includes the description of base64, which is
a good addition, still not consistent with the rest. A paragraph
after all the functions listed is fine I think as the description is
long so it would bloat the table if included directly.
--
Michael

On Wed, 6 Mar 2019 11:27:38 +0900
Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Mar 05, 2019 at 07:55:22PM -0600, Karl O. Pinc wrote:

Attached: doc_base64_v4.patch

Details about the "escape" mode are already available within the
description of function "encode". Wouldn't we want to consolidate a
description for all the modes at the same place, including some words
for hex? Your patch only includes the description of base64, which is
a good addition, still not consistent with the rest. A paragraph
after all the functions listed is fine I think as the description is
long so it would bloat the table if included directly.

Makes sense. (As did hyperlinking to the RFC.)

(No matter how simple I think a patch is going to be it
always turns into a project. :)

Attached: doc_base64_v5.patch

Made index entries for hex and escape encodings.

Added word "encoding" to index entries.

Made <varlist> entries with terms for
base64, hex, and escape encodings.

Added documentation for hex and escape encodings,
including output formats and what are acceptable
inputs.

Regards,

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

On Tue, 5 Mar 2019 23:23:20 -0600
"Karl O. Pinc" <kop@meme.com> wrote:

Added documentation for hex and escape encodings,
including output formats and what are acceptable
inputs.

Attached: doc_base64_v6.patch

Added index entries for encode and decode functions
above the encoding documentation. As index entries
are currently generated this does not make any
difference. But this paragraph is very far
from the other encode/decode index targets on
the page.

Regards,

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

On Wed, 6 Mar 2019 07:09:48 -0600
"Karl O. Pinc" <kop@meme.com> wrote:

On Tue, 5 Mar 2019 23:23:20 -0600
"Karl O. Pinc" <kop@meme.com> wrote:

Added documentation for hex and escape encodings,
including output formats and what are acceptable
inputs.

Attached: doc_base64_v7.patch

Improved escape decoding sentence.

Regards,

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

Attached: doc_base64_v7.patch

Patch applies cleanly, doc compiles, navigation tested and ok.

"... section 6.8" -> "... Section 6.8" (capital S).

"The string and the binary encode and decode functions..." sentence looks
strange to me, especially with the English article that I do not really
master, so maybe it is ok. I'd have written something more
straightforward, eg: "Functions encode and decode support the following
encodings:", and also I'd use a direct "Function <...>decode</...> ..."
rather than "The <function>decode</function> function ..." (twice).

Maybe I'd use the exact same grammatical structure for all 3 cases,
starting with "The <>whatever</> encoding converts bla bla bla" instead of
varying the sentences.

Otherwise, all explanations look both precise and useful to me.

--
Fabien.

On Wed, 6 Mar 2019 19:30:16 +0100 (CET)
Fabien COELHO <coelho@cri.ensmp.fr> wrote:

"... section 6.8" -> "... Section 6.8" (capital S).

Fixed.

"The string and the binary encode and decode functions..." sentence
looks strange to me, especially with the English article that I do
not really master, so maybe it is ok. I'd have written something more
straightforward, eg: "Functions encode and decode support the
following encodings:",

It is an atypical construction because I want to draw attention that
this is documentation not only for the encode() and decode() in
section 9.4. String Functions and Operators but also for
the encode() and decode in section 9.5. Binary String Functions
and Operators. Although I can't think of a better approach
it makes me uncomfortable that documentation written in
one section applies equally to functions in a different section.

Do you think it would be useful to hyperlink the word "binary"
to section 9.5?

The idiomatic phrasing would be "Both the string and the binary
encode and decode functions..." but the word "both" adds
no information. Shorter is better.

and also I'd use a direct "Function
<...>decode</...> ..." rather than "The <function>decode</function>
function ..." (twice).

The straightforward English would be "Decode accepts...". The problem
is that this begins the sentence with the name of a function.
This does not work very well when the function name is all lower case,
and can have other problems where clarity is lost depending
on documentation output formatting.

I don't see a better approach.

Maybe I'd use the exact same grammatical structure for all 3 cases,
starting with "The <>whatever</> encoding converts bla bla bla"
instead of varying the sentences.

Agreed. Good idea. The first paragraph of each term has to
do with encoding and the second with decoding.
Uniformity in starting the second paragraphs helps make
this clear, even though the first paragraphs are not uniform.
With this I am not concerned that the first paragraphs
do not have a common phrasing that's very explicit about
being about encoding.

Adjusted.

Otherwise, all explanations look both precise and useful to me.

When writing I was slightly concerned about being overly precise;
permanently committing to behavior that might (possibly) be an artifact
of implementation. E.g., that hex decoding accepts both
upper and lower case A-F characters, what input is ignored
and what raises an error, etc. But it seems best
to document existing behavior, all of which has existed so long
anyway that changing it would be disruptive. If anybody cares
they can object.

I wrote the docs by reading the code and did only a little
actual testing to be sure that what I wrote is correct.
I also did not check for regression tests which confirm
the behavior I'm documenting. (It wouldn't hurt to have
such regression tests, if they don't already exist.
But writing regression tests is more than I want to take on
with this patch. Feel free to come up with tests. :-)

I'm confident that the behavior I documented is how PG behaves
but you should know what I did in case you want further
validation.

Attached: doc_base64_v8.patch

Regards,

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

Hi Fabien (and Michael),

On Wed, 6 Mar 2019 16:37:05 -0600
"Karl O. Pinc" <kop@meme.com> wrote:

I'm confident that the behavior I documented is how PG behaves
but you should know what I did in case you want further
validation.

Attached: doc_base64_v8.patch

FYI. To avoid a stall in the patch submission process.

I notice that nobody has signed up as a reviewer for
this patch. When the patch looks "ready" it needs
to be marked as such at the PG commitfest website
and a committer will consider committing.

The commitfest URL is:

https://commitfest.postgresql.org/23/

No rush.

Regards,

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

Hello Karl,

I registered as a reviewer in the CF app.

"The string and the binary encode and decode functions..." sentence
looks strange to me, especially with the English article that I do
not really master, so maybe it is ok. I'd have written something more
straightforward, eg: "Functions encode and decode support the
following encodings:",

It is an atypical construction because I want to draw attention that
this is documentation not only for the encode() and decode() in section
9.4. String Functions and Operators but also for the encode() and decode
in section 9.5. Binary String Functions and Operators. Although I can't
think of a better approach it makes me uncomfortable that documentation
written in one section applies equally to functions in a different
section.

People coming from the binary doc would have no reason to look at the
string paragraph anyway.

Do you think it would be useful to hyperlink the word "binary"
to section 9.5?

Hmmm... I think that the link is needed in the other direction.

I'd suggest (1) to use a simpler and direct sentence in the string
section, (2) to simplify/shorten the in cell description in the binary
section, and (3) to add an hyperlink from the binary section which would
point to the expanded explanation in the string section.

The idiomatic phrasing would be "Both the string and the binary
encode and decode functions..." but the word "both" adds
no information. Shorter is better.

Possibly, although "Both" would insist on the fact that it applies to the
two variants, which was your intention.

and also I'd use a direct "Function
<...>decode</...> ..." rather than "The <function>decode</function>
function ..." (twice).

The straightforward English would be "Decode accepts...". The problem
is that this begins the sentence with the name of a function.
This does not work very well when the function name is all lower case,
and can have other problems where clarity is lost depending
on documentation output formatting.

Yep.

I don't see a better approach.

I suggested "Function <>decode</> ...", which is the kind of thing we do
in academic writing to improve precision, because I thought it could be
better:-)

Maybe I'd use the exact same grammatical structure for all 3 cases,
starting with "The <>whatever</> encoding converts bla bla bla"
instead of varying the sentences.

Agreed. Good idea. The first paragraph of each term has to
do with encoding and the second with decoding.

Uniformity in starting the second paragraphs helps make
this clear, even though the first paragraphs are not uniform.
With this I am not concerned that the first paragraphs
do not have a common phrasing that's very explicit about
being about encoding.

Adjusted.

Cannot see it fully in the v8 patch:

- The <literal>base64</literal> encoding is
- <literal>hex</literal> represents
- <literal>escape</literal> converts

Otherwise, all explanations look both precise and useful to me.

When writing I was slightly concerned about being overly precise;

Hmmm. That is a technical documentation, a significant degree of precision
is expected.

--
Fabien.

Hi Fabien,

On Sun, 10 Mar 2019 08:15:35 +0100 (CET)
Fabien COELHO <coel

I registered as a reviewer in the CF app.

Thanks.

What's causing problems here is that the encode and decode
functions are listed in both the string functions section
and the binary functions section. A related but not-relevant
problem is that there are functions listed in the string
function section which take binary input.

I asked about this on IRC and the brief reply was
unflattering to the existing documentation.

So I'm going to fix this also. 3 patches attached:

doc_base64_part1_v9.patch

This moves functions taking bytea and other non-string
input into the binary string section, and vice versa.
Eliminates duplicate encode() and decode() documentation.

Affects: convert(bytea, name, name)
convert_from(bytea, name)
encode(bytea, text)
length(bytea, name)
quote_nullable(anytype)
to_hex(int or bigint)
decode(text, text)

Only moves, eliminates duplicates, and adjusts indentation.

doc_base64_part2_v9.patch

Cleanup wording after moving functions between sections.

doc_base64_part3_v9.patch

Documents base64, hex, and escape encode() and decode()
formats.

"The string and the binary encode and decode functions..." sentence
looks strange to me, especially with the English article that I do
not really master, so maybe it is ok. I'd have written something
more straightforward, eg: "Functions encode and decode support the
following encodings:",

It is an atypical construction because I want to draw attention
that this is documentation not only for the encode() and decode()
in section 9.4. String Functions and Operators but also for the
encode() and decode in section 9.5. Binary String Functions and
Operators. Although I can't think of a better approach it makes me
uncomfortable that documentation written in one section applies
equally to functions in a different section.

People coming from the binary doc would have no reason to look at the
string paragraph anyway.

Do you think it would be useful to hyperlink the word "binary"
to section 9.5?

Hmmm... I think that the link is needed in the other direction.

I'm not sure what you mean here or if it's still relevant.

I'd suggest (1) to use a simpler and direct sentence in the string
section, (2) to simplify/shorten the in cell description in the
binary section, and (3) to add an hyperlink from the binary section
which would point to the expanded explanation in the string section.

The idiomatic phrasing would be "Both the string and the binary
encode and decode functions..." but the word "both" adds
no information. Shorter is better.

Possibly, although "Both" would insist on the fact that it applies to
the two variants, which was your intention.

I think this is no longer relevant. Although I'm not sure what
you mean by 3. The format names already hyperlink back to the
string docs.

and also I'd use a direct "Function
<...>decode</...> ..." rather than "The <function>decode</function>
function ..." (twice).

The straightforward English would be "Decode accepts...". The
problem is that this begins the sentence with the name of a
function. This does not work very well when the function name is
all lower case, and can have other problems where clarity is lost
depending on documentation output formatting.

Yep.

I don't see a better approach.

I suggested "Function <>decode</> ...", which is the kind of thing we
do in academic writing to improve precision, because I thought it
could be better:-)

"Function <>decode</> ..." just does not work in English.

Maybe I'd use the exact same grammatical structure for all 3 cases,
starting with "The <>whatever</> encoding converts bla bla bla"
instead of varying the sentences.

Agreed. Good idea. The first paragraph of each term has to
do with encoding and the second with decoding.

Uniformity in starting the second paragraphs helps make
this clear, even though the first paragraphs are not uniform.
With this I am not concerned that the first paragraphs
do not have a common phrasing that's very explicit about
being about encoding.

Adjusted.

Cannot see it fully in the v8 patch:

- The <literal>base64</literal> encoding is
- <literal>hex</literal> represents
- <literal>escape</literal> converts

I did only the decode paras. I guess no reason not to make
the first paras uniform as well. Done.

I also alphabetized by format name.

I hope that 3 patches will make review easier.

Regards,

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

Er, ping. Nobody has reviewed the latest patchs.
They still apply to master...

I am re-attaching the patches. See descriptions
below.

On Mon, 11 Mar 2019 15:32:14 -0500
"Karl O. Pinc" <kop@meme.com> wrote:

On Sun, 10 Mar 2019 08:15:35 +0100 (CET)
Fabien COELHO <coel

What's causing problems here is that the encode and decode
functions are listed in both the string functions section
and the binary functions section. A related but not-relevant
problem is that there are functions listed in the string
function section which take binary input.

I asked about this on IRC and the brief reply was
unflattering to the existing documentation.

So I'm going to fix this also. 3 patches attached:

doc_base64_part1_v9.patch

This moves functions taking bytea and other non-string
input into the binary string section, and vice versa.
Eliminates duplicate encode() and decode() documentation.

Affects: convert(bytea, name, name)
convert_from(bytea, name)
encode(bytea, text)
length(bytea, name)
quote_nullable(anytype)
to_hex(int or bigint)
decode(text, text)

Only moves, eliminates duplicates, and adjusts indentation.

doc_base64_part2_v9.patch

Cleanup wording after moving functions between sections.

doc_base64_part3_v9.patch

Documents base64, hex, and escape encode() and decode()
formats.

"The string and the binary encode and decode functions..."
sentence looks strange to me, especially with the English
article that I do not really master, so maybe it is ok. I'd have
written something more straightforward, eg: "Functions encode
and decode support the following encodings:",

It is an atypical construction because I want to draw attention
that this is documentation not only for the encode() and decode()
in section 9.4. String Functions and Operators but also for the
encode() and decode in section 9.5. Binary String Functions and
Operators. Although I can't think of a better approach it makes me
uncomfortable that documentation written in one section applies
equally to functions in a different section.

People coming from the binary doc would have no reason to look at
the string paragraph anyway.

Do you think it would be useful to hyperlink the word "binary"
to section 9.5?

Hmmm... I think that the link is needed in the other direction.

I'm not sure what you mean here or if it's still relevant.

I'd suggest (1) to use a simpler and direct sentence in the string
section, (2) to simplify/shorten the in cell description in the
binary section, and (3) to add an hyperlink from the binary section
which would point to the expanded explanation in the string section.

The idiomatic phrasing would be "Both the string and the binary
encode and decode functions..." but the word "both" adds
no information. Shorter is better.

Possibly, although "Both" would insist on the fact that it applies
to the two variants, which was your intention.

I think this is no longer relevant. Although I'm not sure what
you mean by 3. The format names already hyperlink back to the
string docs.

and also I'd use a direct "Function
<...>decode</...> ..." rather than "The
<function>decode</function> function ..." (twice).

The straightforward English would be "Decode accepts...". The
problem is that this begins the sentence with the name of a
function. This does not work very well when the function name is
all lower case, and can have other problems where clarity is lost
depending on documentation output formatting.

Yep.

I don't see a better approach.

I suggested "Function <>decode</> ...", which is the kind of thing
we do in academic writing to improve precision, because I thought it
could be better:-)

"Function <>decode</> ..." just does not work in English.

Maybe I'd use the exact same grammatical structure for all 3
cases, starting with "The <>whatever</> encoding converts bla
bla bla" instead of varying the sentences.

Agreed. Good idea. The first paragraph of each term has to
do with encoding and the second with decoding.

Uniformity in starting the second paragraphs helps make
this clear, even though the first paragraphs are not uniform.
With this I am not concerned that the first paragraphs
do not have a common phrasing that's very explicit about
being about encoding.

Adjusted.

Cannot see it fully in the v8 patch:

- The <literal>base64</literal> encoding is
- <literal>hex</literal> represents
- <literal>escape</literal> converts

I did only the decode paras. I guess no reason not to make
the first paras uniform as well. Done.

I also alphabetized by format name.

I hope that 3 patches will make review easier.

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

Er, ping. Nobody has reviewed the latest patchs.

Next CF is in July, two months away.

You might consider reviewing other people patches, that is expected to
make the overall process work. There are several documentation or
comment patches in the queue.

--
Fabien.

On Thu, 9 May 2019 06:50:12 +0200 (CEST)
Fabien COELHO <coelho@cri.ensmp.fr> wrote:

You might consider reviewing other people patches, that is expected
to make the overall process work. There are several documentation or
comment patches in the queue.

Understood.

I thought I had built up some reviewing credit, from some time
ago. But perhaps that just made up for previous patches.

Regards,

Karl <kop@meme.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

Hello Karl,

doc_base64_part1_v9.patch
Only moves, eliminates duplicates, and adjusts indentation.

doc_base64_part2_v9.patch
Cleanup wording after moving functions between sections.

doc_base64_part3_v9.patch
Documents base64, hex, and escape encode() and decode()
formats.

I suggested "Function <>decode</> ...", which is the kind of thing we
do in academic writing to improve precision, because I thought it
could be better:-)

"Function <>decode</> ..." just does not work in English.

It really works in research papers: "Theorem X can be proven by applying
Proposition Y. See Figure 2 for details. Algorithm Z describes whatever,
which is listed in Table W..."

I also alphabetized by format name.

Good:-)

I hope that 3 patches will make review easier.

Not really. I'm reviewing the 3 patches put together rather than each one
individually, which would require more work.

Patch applies cleanly. Doc build ok.

I looked at the html output, and it seems ok, including navigating to
conversions or formats explanations.

This documentation patch is an overall improvement and clarifies things,
including some error conditions.

convert: I'd merge the 2 first sentences to state that if convert from X
to Y. The doc does not say explicitely what happens if a character cannot
be converted. After testing, an error is raised. The example comment could
add ", if possible".

to_hex: add "." at the end of the sentence?

Other descriptions seem ok.

Minor comment: you usually put two spaces between a "." and the first
world of then next sentence, but not always.

--
Fabien.

Hi Fabien,

Attached is doc_base64_v10.patch

On Fri, 12 Jul 2019 15:58:21 +0200 (CEST)
Fabien COELHO <coelho@cri.ensmp.fr> wrote:

I suggested "Function <>decode</> ...", which is the kind of thing
we do in academic writing to improve precision, because I thought
it could be better:-)

"Function <>decode</> ..." just does not work in English.

It really works in research papers: "Theorem X can be proven by
applying Proposition Y. See Figure 2 for details. Algorithm Z
describes whatever, which is listed in Table W..."

I've not thought about it before but I suppose the difference
is between declarative and descriptive, the latter being
more inviting and better allows for flow between sentences.
Otherwise you're writing in bullet points. So it is a
question of balance between specification and narration.
In regular prose you're always going to see the "the"
unless the sentence starts with the name. The trouble
is that we can't start sentences with function names
because of capitalization confusion.

I hope that 3 patches will make review easier.

Not really. I'm reviewing the 3 patches put together rather than each
one individually, which would require more work.

I figured with e.g. a separate patch for moving and alphabetizing
that it'd be easier to check that nothing got lost. Anyhow,
Just one patch this time.

convert: I'd merge the 2 first sentences to state that if convert
from X to Y. The doc does not say explicitely what happens if a
character cannot be converted. After testing, an error is raised. The
example comment could add ", if possible".

Done. Good idea. I reworked the whole paragraph to shorten and
clarify since I was altering it anyway. This does introduce
some inconsistency with wording that appears elsewhere but it seems
worth it because the listentry box was getting overfull.

to_hex: add "." at the end of the sentence?

I left as-is, without a ".". The only consistent rule about
periods in the listentrys seems to be that if there's more than
one sentence then there's periods -- I think. In any case a
lot of them don't have periods and probably don't need
periods. I don't know what to do and since the original did
not have a period it seems better to leave well enough alone.

Minor comment: you usually put two spaces between a "." and the first
world of then next sentence, but not always.

I now always put 2 spaces after the end of a sentence. But
I've only done this where I've changed text, not when
moving pre-existing text around. Again, there seems
to be no consistency in the original. (I believe docbook
redoes all inter-sentence spacing anyway.)

Thanks for the help.

I'll be sure to sign up to review a patch (or patches) when life
permits.

Regards,

Karl <kop@karlpinc.com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein