Re: libpq parameter parsing problem

On Mon, Jan 13, 2020 at 11:32 PM Jobin Augustine <jobinau(at)gmail(dot)com> wrote:

> Hi Michael,
>
> On Tue, Jan 14, 2020 at 11:33 AM Michael Paquier <michael(at)paquier(dot)xyz>
> wrote:
>
>> On Mon, Jan 13, 2020 at 08:07:06PM +0530, Jobin Augustine wrote:
>> > Sorry for the late reply. Attaching a patch.
>>
>> > <para>
>> > - Percent-encoding may be used to include symbols with special
>> meaning in any
>> > - of the <acronym>URI</acronym> parts, e.g. replace
>> <literal>=</literal> with
>> > - <literal>%3D</literal>.
>> > -
>> > + Connection <acronym>URI</acronym> need to be encoded with
>> > + <ulink url="https://en.wikipedia.org/wiki/Percent-encoding">Percent-encoding</ulink>
>>
>> > + if it includes symbols with special meaning in any of its parts.
>> > + For example:
>> > +<programlisting>
>> > +postgresql://postgres(at)localhost
>> :5432/postgres?options=-c%20synchronous_commit%3Doff%20-c%20geqo%3Doff
>> > +</programlisting>
>> > + where all <literal>=</literal> are replaced with
>> <literal>%3D</literal> and
>> > + space character with <literal>%20</literal>
>> > </para>
>>
>> The reference to wikipedia is nice to have. A small nit from me is
>> that I would group the last sentence with the "For example", to give:
>> "Here is an example where all = are replaced.."
>>
> Yes, agree, readability is better with that modification.
> Attaching a modified patch.
>
>> The first sentence sounds good to me.
>>
>>
Should start out: "The Connection URI needs to be encoded with (confirm
OK-ness of wikipedia direct link in docs) Percent-encoding..." (mainly
"need => needs", the "the" reads better to me though)

I would probably choose to move the example for the options parameters to
the "Parameter Key Words" options section:

...represent a literal backslash.</p>

<p>When specifying options as part of a percent-encoded Connection URI the
structural space(s) and equal sign(s) "=" need to be encoded as %20 and %3D
respectively (in addition to any value-specific encoding needs) while the
escaping back-slash "\" does not. For example:
<programlisting>postgresql:///mydb?options=-c%20synchronous_commit%3Doff%20-c%20geqo%3Doff</programlisting>

<p>For a detailed...

If you'd like an example for the first section maybe something like having
a space in your database name:

postgresql://user(at)localhost:5432/my%20database

Also, regardless of where it is placed having both the username and
database both be named "postgres" in an example just adds unnecessary
mental effort to understanding the example. One that none of the existing
examples do. Name your user "user" and database "mydb" unless, as with the
desire to include a space, there is a meaningful reason not to.

Also, in my modified example (for options) above since everything is
optional omitting everything except the database and parameters removes
noise (though keeping them may increase comfort).

David J.