Re: Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used

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

In response to

Responses

Browse pgsql-hackers by date

  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