From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
Cc: | Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used |
Date: | 2013-01-28 15:44:54 |
Message-ID: | CAFj8pRDbDmtrptgX8iqq2M3+X=xNHHRXrOLT1t+3YkeG7f-tYQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello
2013/1/28 Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>:
> On 26 January 2013 10:58, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> updated patches due changes for better variadic "any" function.
>>
>> apply fix_mixing_positinal_ordered_placeholders_warnings_20130126.patch first
>>
>
> Hi,
>
> No one is listed as a reviewer for this patch so I thought I would
> take a look at it, since it looks like a useful enhancement to
> format().
>
> Starting with the first patch - it issues a new WARNING if the format
> string contains a mixture of format specifiers with and without
> parameter indexes (e.g., 'Hello %s, %1$s').
>
> Having thought about it a bit, I really don't like this for a number of reasons:
>
> * I actually quite like the current behaviour. Admittedly putting
> ordered specifiers (like '%s') after positional ones (like '%3$s') is
> probably not so useful, and potentially open to different
> interpretations. But putting positional specifiers at the end is
> completely unambiguous and can save a lot of typing (e.g.,
> '%s,%s,%s,%s,%,s,%s,%s,%1$s').
>
> * On backwards compatibility grounds. The fact that the only example
> of format() in the manual is precisely a case of mixed positional and
> ordered parameters makes it quite likely that people will have used
> this feature already.
>
> * Part of the justification for adding the warning is for
> compatibility with glibc/SUS printf(). But if we are aiming for that,
> then we should also produce a warning if positional parameters are
> used and not all parameters are consumed. That would be a pain to
> implement and I don't think it would be particularly helpful in
> practice. Here is what the SUS says:
>
> """
> The format can contain either numbered argument specifications (that
> is, %n$ and *m$), or unnumbered argument specifications (that is, %
> and *), but normally not both. The only exception to this is that %%
> can be mixed with the %n$ form. The results of mixing numbered and
> unnumbered argument specifications in a format string are undefined.
> When numbered argument specifications are used, specifying the Nth
> argument requires that all the leading arguments, from the first to
> the (N-1)th, are specified in the format string.
> """
>
> I think that if we are going to do anything, we should explicitly
> document our current behaviour as a PostgreSQL extension to the SUS
> printf(), describing how we handle mixed parameters, rather than
> adding this warning.
>
> The current PostgreSQL code isn't inconsistent with the SUS, except in
> the error case, and so can reasonably be regarded as an extension.
>
I am not sure what you dislike?
warnings or redesign of behave.
I can live without warnings, when this field will be documented - it
is not fully compatible with gcc, but gcc just raises warnings and
does correct implementation. Our warnings are on different level than
gcc warnings.
But I don't think so current implementation is correct
-- our current behave
postgres=# select format('%s %2$s %s', 'Hello', 'World');
ERROR: too few arguments for format
postgres=#
postgres=# select format('%s %1$s %s', 'Hello', 'World'); -- works
ordered parameters should be independent on positional parameters. And
this behave has glibc
Regards
Pavel
> Regards,
> Dean
From | Date | Subject | |
---|---|---|---|
Next Message | Kevin Grittner | 2013-01-28 16:15:29 | Re: autovacuum not prioritising for-wraparound tables |
Previous Message | Simon Riggs | 2013-01-28 15:40:08 | Re: pg_ctl idempotent option |