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

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I found so we doesn't have functionality for simply text aligning - so
I propose support width for %s like printf's behave. glibc
implementation knows a rule for precision, that I don't would to
implement, because it is oriented to bytes and not to chars - and it
can be confusing. Still I would to have implementation and design of
"format" function maximally simple - and a rule for "s" specifier and
width is clean and simple.

I started looking at this patch to get a head-start on the next
commitfest. There's no documentation, which certainly needs to be
fixed, but worse, this doesn't appear to match glibc printf and it's not
entirely clear to me why it doesn't.

-- our current behave
postgres=# select format('%s %2$s %s', 'Hello', 'World');
ERROR: too few arguments for format
postgres=#

This is correct, if we're matching glibc (and SUS, I believe), isn't it?
You're not allowed to mix '%2$s' type parameters and '%s' in a single
format.

Thanks,

Stephen

Hello

2012/12/29 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I found so we doesn't have functionality for simply text aligning - so
I propose support width for %s like printf's behave. glibc
implementation knows a rule for precision, that I don't would to
implement, because it is oriented to bytes and not to chars - and it
can be confusing. Still I would to have implementation and design of
"format" function maximally simple - and a rule for "s" specifier and
width is clean and simple.

I started looking at this patch to get a head-start on the next
commitfest. There's no documentation, which certainly needs to be
fixed, but worse, this doesn't appear to match glibc printf and it's not
entirely clear to me why it doesn't.

-- our current behave
postgres=# select format('%s %2$s %s', 'Hello', 'World');
ERROR: too few arguments for format
postgres=#

This is correct, if we're matching glibc (and SUS, I believe), isn't it?
You're not allowed to mix '%2$s' type parameters and '%s' in a single
format.

I am not sure, please recheck

pavel ~ $ cat test.c
#include <stdio.h>

void main()
{

printf("%s %2$s %s\n", "AHOJ", "Svete");
}

pavel ~ $ gcc test.c # no warning here
pavel ~ $ gcc --version
gcc (GCC) 4.7.2 20120921 (Red Hat 4.7.2-2)
Copyright (C) 2012 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

pavel ~ $ ./a.out
AHOJ Svete Svete
pavel ~ $

Regards

Pavel Stehule

Thanks,

Stephen

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

2012/12/29 Stephen Frost <sfrost@snowman.net>:

This is correct, if we're matching glibc (and SUS, I believe), isn't it?
You're not allowed to mix '%2$s' type parameters and '%s' in a single
format.

I am not sure, please recheck

According to the man pages on my Ubuntu system, under 'Format of the
format string':

-------------------
If the style using '$' is used, it must be used throughout for
all conversions taking an argument and all width and precision
arguments, but it may be mixed with "%%" formats which do not consume
an argument.
-------------------

pavel ~ $ cat test.c
#include <stdio.h>

void main()
{

printf("%s %2$s %s\n", "AHOJ", "Svete");
}

pavel ~ $ gcc test.c # no warning here

You didn't turn any on...

sfrost@tamriel:/home/sfrost> gcc -o qq -Wall test.c
test.c: In function ‘main’:
test.c:5:3: warning: $ operand number used after format without operand number [-Wformat]

Thanks,

Stephen

2012/12/29 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

2012/12/29 Stephen Frost <sfrost@snowman.net>:

This is correct, if we're matching glibc (and SUS, I believe), isn't it?
You're not allowed to mix '%2$s' type parameters and '%s' in a single
format.

I am not sure, please recheck

According to the man pages on my Ubuntu system, under 'Format of the
format string':

-------------------
If the style using '$' is used, it must be used throughout for
all conversions taking an argument and all width and precision
arguments, but it may be mixed with "%%" formats which do not consume
an argument.
-------------------

pavel ~ $ cat test.c
#include <stdio.h>

void main()
{

printf("%s %2$s %s\n", "AHOJ", "Svete");
}

pavel ~ $ gcc test.c # no warning here

You didn't turn any on...

sfrost@tamriel:/home/sfrost> gcc -o qq -Wall test.c
test.c: In function ‘main’:
test.c:5:3: warning: $ operand number used after format without operand number [-Wformat]

ok, so what is proposed solution?

I see two possibilities - a) applying my current patch - although it
is not fully correct, b) new patch, that do necessary check and raise
more descriptive error message.

I have not strong preferences in this topic - both variants are
acceptable for me and I invite any community opinion. But current
state is not intuitive and should be fixed.

Regards

Pavel

Thanks,

Stephen

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

ok, so what is proposed solution?

My recommendation would be to match what glibc's printf does.

I see two possibilities - a) applying my current patch - although it
is not fully correct, b) new patch, that do necessary check and raise
more descriptive error message.

Right, have a new patch that does error-checking and returns a better
error on that case, update the docs to reflect that restriction, and
then (ideally as an additional and independent patch..) implement the
width capability (and, ideally, the ability to pass the width as an
argument, as glibc supports) which matches the glibc arguments.

Part of the reason that this restriction is in place, I believe, is
because glibc expects the width to come before any explicit argument
being passed and if an explicit argument is used for width then an
explicit argument has to be used for the value also, otherwise it
wouldn't be clear from the format which was the argument number and
which was the explicit width size.

I don't think it's a good idea to come up with our own format
definition, particularly one which looks so similar to the well-known
printf() format.

I have not strong preferences in this topic - both variants are
acceptable for me and I invite any community opinion. But current
state is not intuitive and should be fixed.

Agreed.

Thanks,

Stephen

Hello Stephen

2012/12/29 Stephen Frost <sfrost@snowman.net>:

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

ok, so what is proposed solution?

My recommendation would be to match what glibc's printf does.

I see two possibilities - a) applying my current patch - although it
is not fully correct, b) new patch, that do necessary check and raise
more descriptive error message.

Right, have a new patch that does error-checking and returns a better
error on that case, update the docs to reflect that restriction, and
then (ideally as an additional and independent patch..) implement the
width capability (and, ideally, the ability to pass the width as an
argument, as glibc supports) which matches the glibc arguments.

Part of the reason that this restriction is in place, I believe, is
because glibc expects the width to come before any explicit argument
being passed and if an explicit argument is used for width then an
explicit argument has to be used for the value also, otherwise it
wouldn't be clear from the format which was the argument number and
which was the explicit width size.

I found one issue - if I disallow mixing positional and ordered style
I break compatibility with previous implementation.

so maybe third way is better - use fix from my patch - a behave is
same like in glibc - and raise warning (instead errors) when mixing
styles is detected - we can replace warnings by errors in future.

What do you think?

Regards

Pavel

I don't think it's a good idea to come up with our own format
definition, particularly one which looks so similar to the well-known
printf() format.

I have not strong preferences in this topic - both variants are
acceptable for me and I invite any community opinion. But current
state is not intuitive and should be fixed.

Agreed.

Thanks,

Stephen

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2012/12/30 Pavel Stehule <pavel.stehule@gmail.com>:

Hello Stephen

2012/12/29 Stephen Frost <sfrost@snowman.net>:

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

ok, so what is proposed solution?

My recommendation would be to match what glibc's printf does.

I see two possibilities - a) applying my current patch - although it
is not fully correct, b) new patch, that do necessary check and raise
more descriptive error message.

Right, have a new patch that does error-checking and returns a better
error on that case, update the docs to reflect that restriction, and
then (ideally as an additional and independent patch..) implement the
width capability (and, ideally, the ability to pass the width as an
argument, as glibc supports) which matches the glibc arguments.

Part of the reason that this restriction is in place, I believe, is
because glibc expects the width to come before any explicit argument
being passed and if an explicit argument is used for width then an
explicit argument has to be used for the value also, otherwise it
wouldn't be clear from the format which was the argument number and
which was the explicit width size.

I found one issue - if I disallow mixing positional and ordered style
I break compatibility with previous implementation.

so maybe third way is better - use fix from my patch - a behave is
same like in glibc - and raise warning (instead errors) when mixing
styles is detected - we can replace warnings by errors in future.

this is exactly what gcc does - and without breaking applications.

What do you think?

Regards

Pavel

I don't think it's a good idea to come up with our own format
definition, particularly one which looks so similar to the well-known
printf() format.

I have not strong preferences in this topic - both variants are
acceptable for me and I invite any community opinion. But current
state is not intuitive and should be fixed.

Agreed.

Thanks,

Stephen

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I found one issue - if I disallow mixing positional and ordered style
I break compatibility with previous implementation.

Can you elaborate? In the previous example, an error was returned when
mixing (not a terribly good one, but still an error). Returning a
better error won't be a problem.

so maybe third way is better - use fix from my patch - a behave is
same like in glibc - and raise warning (instead errors) when mixing
styles is detected - we can replace warnings by errors in future.

What do you think?

If there are cases which work today then I agree that we should issue a
warning to avoid breaking existing applications. We should still use
the glibc format when adding width support, however.

Thanks,

Stephen

Hello

2012/12/31 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I found one issue - if I disallow mixing positional and ordered style
I break compatibility with previous implementation.

Can you elaborate? In the previous example, an error was returned when
mixing (not a terribly good one, but still an error). Returning a
better error won't be a problem.

A result from ours previous talk was a completely disabling mixing
positional and ordered placeholders - like is requested by man and gcc
raises warnings there.

But mixing is not explicitly disallowed in doc, and mixing was tested
in our regress tests. There are tests where placeholders are mixed -
so anybody can use it.

select format('Hello %s %1$s %s', 'World', 'Hello again'); -- is
enabled and supported and result is expected

-- but this raises error - and it is same situation like previous example
select format('%s %2$s %s', 'Hello', 'World');

-- so bot examples should be executed or should be disabled if this
functionality should be consistent. And I can't to break first
example, then I have to repair second example

Regards

so maybe third way is better - use fix from my patch - a behave is
same like in glibc - and raise warning (instead errors) when mixing
styles is detected - we can replace warnings by errors in future.

What do you think?

If there are cases which work today then I agree that we should issue a
warning to avoid breaking existing applications. We should still use
the glibc format when adding width support, however.

Thanks,

Stephen

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

A result from ours previous talk was a completely disabling mixing
positional and ordered placeholders - like is requested by man and gcc
raises warnings there.

But mixing is not explicitly disallowed in doc, and mixing was tested
in our regress tests. There are tests where placeholders are mixed -
so anybody can use it.
select format('Hello %s %1$s %s', 'World', 'Hello again'); -- is
enabled and supported and result is expected

Alright, then I agree that raising a warning in that case makes sense
and let's update the docs to reflect that it shouldn't be done (like
what glibc/gcc do).

Thanks,

Stephen

2012/12/31 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

A result from ours previous talk was a completely disabling mixing
positional and ordered placeholders - like is requested by man and gcc
raises warnings there.

But mixing is not explicitly disallowed in doc, and mixing was tested
in our regress tests. There are tests where placeholders are mixed -
so anybody can use it.
select format('Hello %s %1$s %s', 'World', 'Hello again'); -- is
enabled and supported and result is expected

Alright, then I agree that raising a warning in that case makes sense
and let's update the docs to reflect that it shouldn't be done (like
what glibc/gcc do).

ok, I prepare patch

Regards

Pavel

Thanks,

Stephen

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello

2012/12/31 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

A result from ours previous talk was a completely disabling mixing
positional and ordered placeholders - like is requested by man and gcc
raises warnings there.

But mixing is not explicitly disallowed in doc, and mixing was tested
in our regress tests. There are tests where placeholders are mixed -
so anybody can use it.
select format('Hello %s %1$s %s', 'World', 'Hello again'); -- is
enabled and supported and result is expected

Alright, then I agree that raising a warning in that case makes sense
and let's update the docs to reflect that it shouldn't be done (like
what glibc/gcc do).

so there are two patches - first is fix in logic when positional and
ordered parameters are mixed + add warning in this situation. Second
patch enables possibility to specify width for %s conversion.

I didn't finalize documentation due my net good English skills -
probably there is necessary new paragraph about function "format"
elsewhere than in table

Regards

Pavel

Hello

2012/12/31 Pavel Stehule <pavel.stehule@gmail.com>:

Hello

2012/12/31 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

A result from ours previous talk was a completely disabling mixing
positional and ordered placeholders - like is requested by man and gcc
raises warnings there.

But mixing is not explicitly disallowed in doc, and mixing was tested
in our regress tests. There are tests where placeholders are mixed -
so anybody can use it.
select format('Hello %s %1$s %s', 'World', 'Hello again'); -- is
enabled and supported and result is expected

Alright, then I agree that raising a warning in that case makes sense
and let's update the docs to reflect that it shouldn't be done (like
what glibc/gcc do).

so there are two patches - first is fix in logic when positional and
ordered parameters are mixed + add warning in this situation. Second
patch enables possibility to specify width for %s conversion.

I didn't finalize documentation due my net good English skills -
probably there is necessary new paragraph about function "format"
elsewhere than in table

Regards

Pavel

updated patches due changes for better variadic "any" function.

apply fix_mixing_positinal_ordered_placeholders_warnings_20130126.patch first

Regards

Pavel

On 26 January 2013 10:58, Pavel Stehule <pavel.stehule@gmail.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.

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hello

2013/1/28 Dean Rasheed <dean.a.rasheed@gmail.com>:

On 26 January 2013 10:58, Pavel Stehule <pavel.stehule@gmail.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Pavel Stehule <pavel.stehule@gmail.com> writes:

2013/1/28 Dean Rasheed <dean.a.rasheed@gmail.com>:

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 am not sure what you dislike?
warnings or redesign of behave.

Both. If we had done this when we first implemented format(), it'd be
fine, but it's too late to change it now. There very likely are
applications out there that depend on the current behavior. As Dean
says, it's not incompatible with SUS, just a superset, so ISTM this
patch is proposing to remove documented functionality --- for no very
strong reason.

I vote for rejecting this change entirely.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Both. If we had done this when we first implemented format(), it'd be
fine, but it's too late to change it now. There very likely are
applications out there that depend on the current behavior. As Dean
says, it's not incompatible with SUS, just a superset, so ISTM this
patch is proposing to remove documented functionality --- for no very
strong reason.

It's only a "superset" of the very poor subset of printf()-like
functionality that we currently support through the format() function.

If we can actually match glibc/SUS (which I don't believe the initial
patch did..) and support a mix of explicitly specified arguments and
implicit arguments, along with the various width, precision, and other
format specifications, then fine by me.

I'm not convinced that's actually possible due to the ambiguity which
will certainly arise and I'm quite sure the documentation that explains
what we do in each case will deserve it's own chapter.

Thanks,

Stephen

On 28 January 2013 17:32, Stephen Frost <sfrost@snowman.net> wrote:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Both. If we had done this when we first implemented format(), it'd be
fine, but it's too late to change it now. There very likely are
applications out there that depend on the current behavior. As Dean
says, it's not incompatible with SUS, just a superset, so ISTM this
patch is proposing to remove documented functionality --- for no very
strong reason.

It's only a "superset" of the very poor subset of printf()-like
functionality that we currently support through the format() function.

If we can actually match glibc/SUS (which I don't believe the initial
patch did..) and support a mix of explicitly specified arguments and
implicit arguments, along with the various width, precision, and other
format specifications, then fine by me.

I'm not convinced that's actually possible due to the ambiguity which
will certainly arise and I'm quite sure the documentation that explains
what we do in each case will deserve it's own chapter.

There are a number of separate issues here, but I don't see this as an
intractable problem. In general a format specifier looks like:

%[parameter][flags][width][.precision][length]type

parameter - an optional n$. This is where we have implemented a
superset of the SUS printf(). But I think it is a useful superset, and
it's too late to remove it now. Any ambiguity lies here, where we go
beyond the SUS - some printf() implementations appear to do something
different (apparently without documenting what they do). I think our
documentation could be clearer here, to explain how mixed parameters
are handled.

flags - not currently implemented. Pavel's second patch adds support
for the '-' flag for left justified string output. However, I think
this should support all datatypes (i.e., %I and %L as well as %s).

width - not currently implemented. Pavel's second patch adds support
for this, but note that for full compatibility with the SUS it needs
to also support widths specified using * and *n$. Also, I think it
should support all supported datatypes, not just strings.

precision - only relevant to numeric datatypes, which we don't support.

length - only relevant to numeric datatypes, which we don't support.

type - this is where we only support a small subset of the SUS (plus a
couple of SQL-specific types). I'm not sure if anyone has any plans to
extend this, but that's certainly not on the cards for 9.3.

So the relevant pieces that Pavel's second patch is attempting to add
support for are the '-' flag and the width field. As noted above,
there are a couple of areas where the current patch falls short of the
SUS:

1). The '-' flag and width field are supposed to apply to all types. I
think that not supporting %I and %L will be somewhat limiting, and
goes against the intent of the SUS, even though those types are
PostgreSQL extensions.

2). The width field is supposed to support * (width specified by the
next function argument) and *n$ (width specified by the nth function
argument). If we supported this, then we could claim full
compatibility with the SUS in all fields except for the type support,
which would seem like a real step forward.

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2013/1/28 Dean Rasheed <dean.a.rasheed@gmail.com>:

On 28 January 2013 17:32, Stephen Frost <sfrost@snowman.net> wrote:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Both. If we had done this when we first implemented format(), it'd be
fine, but it's too late to change it now. There very likely are
applications out there that depend on the current behavior. As Dean
says, it's not incompatible with SUS, just a superset, so ISTM this
patch is proposing to remove documented functionality --- for no very
strong reason.

It's only a "superset" of the very poor subset of printf()-like
functionality that we currently support through the format() function.

If we can actually match glibc/SUS (which I don't believe the initial
patch did..) and support a mix of explicitly specified arguments and
implicit arguments, along with the various width, precision, and other
format specifications, then fine by me.

I'm not convinced that's actually possible due to the ambiguity which
will certainly arise and I'm quite sure the documentation that explains
what we do in each case will deserve it's own chapter.

There are a number of separate issues here, but I don't see this as an
intractable problem. In general a format specifier looks like:

%[parameter][flags][width][.precision][length]type

parameter - an optional n$. This is where we have implemented a
superset of the SUS printf(). But I think it is a useful superset, and
it's too late to remove it now. Any ambiguity lies here, where we go
beyond the SUS - some printf() implementations appear to do something
different (apparently without documenting what they do). I think our
documentation could be clearer here, to explain how mixed parameters
are handled.

flags - not currently implemented. Pavel's second patch adds support
for the '-' flag for left justified string output. However, I think
this should support all datatypes (i.e., %I and %L as well as %s).

no - surely not - I% and L% is PostgreSQL extension and left or right
alignment is has no sense for PostgreSQL identifiers and PostgreSQL
literals.

Regards

Pavel

width - not currently implemented. Pavel's second patch adds support
for this, but note that for full compatibility with the SUS it needs
to also support widths specified using * and *n$. Also, I think it
should support all supported datatypes, not just strings.

precision - only relevant to numeric datatypes, which we don't support.

length - only relevant to numeric datatypes, which we don't support.

type - this is where we only support a small subset of the SUS (plus a
couple of SQL-specific types). I'm not sure if anyone has any plans to
extend this, but that's certainly not on the cards for 9.3.

So the relevant pieces that Pavel's second patch is attempting to add
support for are the '-' flag and the width field. As noted above,
there are a couple of areas where the current patch falls short of the
SUS:

1). The '-' flag and width field are supposed to apply to all types. I
think that not supporting %I and %L will be somewhat limiting, and
goes against the intent of the SUS, even though those types are
PostgreSQL extensions.

2). The width field is supposed to support * (width specified by the
next function argument) and *n$ (width specified by the nth function
argument). If we supported this, then we could claim full
compatibility with the SUS in all fields except for the type support,
which would seem like a real step forward.

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers