On Mon, Feb 15, 2016 at 12:57 AM, Andres Freund <andres@anarazel.de> wrote:
> On 2016-02-14 10:51:01 -0500, Tom Lane wrote:
>> Michael Paquier <michael.paquier@gmail.com> writes:
>> > Commas are authorized characters in application_name for a node in
>> > recovery, however this overlaps with the fact that
>> > synchronous_standby_names uses commas as a separator for each node
>> > name. So, if a standby node uses a comma in its name, even if its name
>> > is set in s_s_names it can never become a synchronous node because
>> > SplitIdentifierString() splits this parameter with only a comma.
>>
>> > Even if I have never seen an standby using a comma in its
>> > application_name, this is a bug, and here are a couple of things that
>> > we could do regarding it:
>> > 1) Do not care, who is actually going to use a comma in application_name?!
>> > 2) Forbid the use of commas in application_name
>> > 3) Enhaunce a bit s_s_names splitting so as it can consider
>> > backslash+comma as part of a standby name. In short by setting
>> > s_s_names = 'foo\,bar', a standby with name 'foo,bar' would be a sync
>> > node.
>> > 4) Document the limitation and discourage the use of commas in application_name
>>
>> (3) seems like a mess with likely side-effects on other uses of
>> SplitIdentifierString. I'd vote for (2) or (4).
>
> I know a number of applications written by clients, without my
> encouragement, that use application name to provide e.g. the application
> user triggering queries. At least two had used commas at some point. So
> I'm disinclined to go with 2).
Another possibility to consider would be to mention that in the
documentation on back-branches, and simply block its use on HEAD. For
the quorum sync patch (which is far from baked yet), we are going to
need two extra special characters to identify quorum and priority
groups of nodes, so we would need to provide the same solution as for
commas.
--
Michael