Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Thank you for committing this patch. Applied with some mostly-cosmetic adjustments. I also took the liberty of changing some of the error message texts to line up more closely with the expanded documentation (eg, use format specifier not conversion specifier because that's the phrase used in the docs). I looked over the modifications. Thanks for refining rather large portion of documentation and comments.. and code. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp writes: [ format-width-20130305.patch ] Applied with some mostly-cosmetic adjustments. I also took the liberty of changing some of the error message texts to line up more closely with the expanded documentation (eg, use format specifier not conversion specifier because that's the phrase used in the docs). 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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello, Patches can be reviewed by more than one people. It's particularly useful if they have different things to say. So don't hesitate to review patches that have already been reviewed by other people. Thanks for the advice. I was anxious about who among the reviewers is, and when to make a decisision if the patch has reached the level or not, I'll take it more easy. In fact, you can even review committed patches; it's not unlikely that you will be able to find bugs in those, too. Umm.. to be sure.. -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello,
I think that the only other behavioural glitch I spotted was that it
fails to catch one overflow case, which should generate an out of
ranger error:
select format('|%*s|', -2147483648, 'foo');
Result: |foo|
because -(-2147483648) = 0 in signed 32-bit integers.
Ouch. Thanks for pointing out.
fixed - next other overflow check added
Your change shown below seems assuming that the two's complement
of the most negative number in integer types is identical to
itself, and looks working as expected at least on
linux/x86_64. But C standard defines it as undefined, (As far as
I hear :-).
| if (width != 0)
| {
| int32 _width = -width;
|
| if (SAMESIGN(width, _width))
| ereport(ERROR,
Instead, I think we can deny it by simply comparing with
INT_MIN. I modified the patch like so and put some modifications
on styling.
Finally, enlargeStringInfo fails just after for large numbers. We
might should keep it under certain length to get rid of memory
exhaustion.
Anyway, I'll send this patch to committers as it is in this
message.
best wishes,
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 9b7e967..b2d2ed6 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -1519,21 +1519,13 @@
primaryformat/primary
/indexterm
literalfunctionformat/function(parameterformatstr/parameter typetext/type
-[, parameterstr/parameter typeany/type [, ...] ])/literal
+[, parameterformatarg/parameter typeany/type [, ...] ])/literal
/entry
entrytypetext/type/entry
entry
Format arguments according to a format string.
- This function is similar to the C function
- functionsprintf/, but only the following conversion specifications
- are recognized: literal%s/literal interpolates the corresponding
- argument as a string; literal%I/literal escapes its argument as
- an SQL identifier; literal%L/literal escapes its argument as an
- SQL literal; literal%%/literal outputs a literal literal%/.
- A conversion can reference an explicit parameter position by preceding
- the conversion specifier with literalreplaceablen/$/, where
- replaceablen/replaceable is the argument position.
- See also xref linkend=plpgsql-quote-literal-example.
+ This function is similar to the C function functionsprintf/.
+ See xref linkend=functions-string-format.
/entry
entryliteralformat('Hello %s, %1$s', 'World')/literal/entry
entryliteralHello World, World/literal/entry
@@ -2847,6 +2839,186 @@
/tgroup
/table
+ sect2 id=functions-string-format
+titlefunctionformat/function/title
+
+indexterm
+ primaryformat/primary
+/indexterm
+
+para
+ The function functionformat/ produces formatted output according to
+ a format string in a similar way to the C function functionsprintf/.
+/para
+
+para
+synopsis
+format(parameterformatstr/ typetext/ [, parameterformatarg/ typeany/ [, ...] ])
+/synopsis
+ replaceableformatstr/ is a format string that specifies how the
+ result should be formatted. Text in the format string is copied directly
+ to the result, except where firsttermformat specifiers/ are used.
+ Format specifiers act as placeholders in the string, allowing subsequent
+ function arguments to be formatted and inserted into the result.
+/para
+
+para
+ Format specifiers are introduced by a literal%/ character and take
+ the form
+synopsis
+%[replaceableparameter/][replaceableflags/][replaceablewidth/]replaceabletype/
+/synopsis
+ variablelist
+ varlistentry
+ termreplaceableparameter/replaceable (optional)/term
+ listitem
+para
+ An expression of the form literalreplaceablen/$/ where
+ replaceablen/ is the index of the argument to use for the format
+ specifier's value. An index of 1 means the first argument after
+ replaceableformatstr/. If the replaceableparameter/ field is
+ omitted, the default is to use the next argument.
+/para
+screen
+SELECT format('Testing %s, %s, %s', 'one', 'two', 'three');
+lineannotationResult: /computeroutputTesting one, two, three/
+
+SELECT format('Testing %3$s, %2$s, %1$s', 'one', 'two', 'three');
+lineannotationResult: /computeroutputTesting three, two, one/
+/screen
+
+para
+ Note that unlike the C function functionsprintf/ defined in the
+ Single UNIX Specification, the functionformat/ function in
+ productnamePostgreSQL/ allows format specifiers with and without
+ explicit replaceableparameter/ fields to be mixed in the same
+ format string. A format specifier without a
+ replaceableparameter/ field always uses the next
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
2013/3/5 Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp:
Hello,
I think that the only other behavioural glitch I spotted was that it
fails to catch one overflow case, which should generate an out of
ranger error:
select format('|%*s|', -2147483648, 'foo');
Result: |foo|
because -(-2147483648) = 0 in signed 32-bit integers.
Ouch. Thanks for pointing out.
fixed - next other overflow check added
Your change shown below seems assuming that the two's complement
of the most negative number in integer types is identical to
itself, and looks working as expected at least on
linux/x86_64. But C standard defines it as undefined, (As far as
I hear :-).
| if (width != 0)
| {
| int32 _width = -width;
|
| if (SAMESIGN(width, _width))
| ereport(ERROR,
this pattern was used elsewhere in pg
Instead, I think we can deny it by simply comparing with
INT_MIN. I modified the patch like so and put some modifications
on styling.
ook - I have not enough expirience with this topic and I cannot say
what is preferred.
Finally, enlargeStringInfo fails just after for large numbers. We
might should keep it under certain length to get rid of memory
exhaustion.
I though about it, but I don't know a correct value - probably any
width specification higher 1MB will be bogus and can be blocked ?? Our
VARLENA max size is 1GB so with should not be higher than 1GB ever.
what do you thinking about these limits?
Regards
Pavel
Anyway, I'll send this patch to committers as it is in this
message.
best wishes,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
On 5 March 2013 13:46, Pavel Stehule pavel.steh...@gmail.com wrote:
2013/3/5 Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp:
Hello,
I think that the only other behavioural glitch I spotted was that it
fails to catch one overflow case, which should generate an out of
ranger error:
select format('|%*s|', -2147483648, 'foo');
Result: |foo|
because -(-2147483648) = 0 in signed 32-bit integers.
Ouch. Thanks for pointing out.
fixed - next other overflow check added
Your change shown below seems assuming that the two's complement
of the most negative number in integer types is identical to
itself, and looks working as expected at least on
linux/x86_64. But C standard defines it as undefined, (As far as
I hear :-).
| if (width != 0)
| {
| int32 _width = -width;
|
| if (SAMESIGN(width, _width))
| ereport(ERROR,
this pattern was used elsewhere in pg
Instead, I think we can deny it by simply comparing with
INT_MIN. I modified the patch like so and put some modifications
on styling.
ook - I have not enough expirience with this topic and I cannot say
what is preferred.
Finally, enlargeStringInfo fails just after for large numbers. We
might should keep it under certain length to get rid of memory
exhaustion.
I though about it, but I don't know a correct value - probably any
width specification higher 1MB will be bogus and can be blocked ?? Our
VARLENA max size is 1GB so with should not be higher than 1GB ever.
what do you thinking about these limits?
I think it's fine as it is.
It's no different from repeat() for example. We allow repeat('a',
10) so allowing format('%10s', '') seems reasonable,
although probably not very useful.
Upping either beyond 1GB generates an out of memory error, which also
seems reasonable -- I can't imagine why you would want such a long
string.
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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
2013/3/5 Dean Rasheed dean.a.rash...@gmail.com:
On 5 March 2013 13:46, Pavel Stehule pavel.steh...@gmail.com wrote:
2013/3/5 Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp:
Hello,
I think that the only other behavioural glitch I spotted was that it
fails to catch one overflow case, which should generate an out of
ranger error:
select format('|%*s|', -2147483648, 'foo');
Result: |foo|
because -(-2147483648) = 0 in signed 32-bit integers.
Ouch. Thanks for pointing out.
fixed - next other overflow check added
Your change shown below seems assuming that the two's complement
of the most negative number in integer types is identical to
itself, and looks working as expected at least on
linux/x86_64. But C standard defines it as undefined, (As far as
I hear :-).
| if (width != 0)
| {
| int32 _width = -width;
|
| if (SAMESIGN(width, _width))
| ereport(ERROR,
this pattern was used elsewhere in pg
Instead, I think we can deny it by simply comparing with
INT_MIN. I modified the patch like so and put some modifications
on styling.
ook - I have not enough expirience with this topic and I cannot say
what is preferred.
Finally, enlargeStringInfo fails just after for large numbers. We
might should keep it under certain length to get rid of memory
exhaustion.
I though about it, but I don't know a correct value - probably any
width specification higher 1MB will be bogus and can be blocked ?? Our
VARLENA max size is 1GB so with should not be higher than 1GB ever.
what do you thinking about these limits?
I think it's fine as it is.
It's no different from repeat() for example. We allow repeat('a',
10) so allowing format('%10s', '') seems reasonable,
although probably not very useful.
Upping either beyond 1GB generates an out of memory error, which also
seems reasonable -- I can't imagine why you would want such a long
string.
Regards,
Dean
ok
Pavel
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Kyotaro HORIGUCHI escribió: Umm. sorry, If you have no problem with this, I'll send this to committer. I just found that this patch already has a revewer. I've seen only Status field in patch list.. Patches can be reviewed by more than one people. It's particularly useful if they have different things to say. So don't hesitate to review patches that have already been reviewed by other people. In fact, you can even review committed patches; it's not unlikely that you will be able to find bugs in those, too. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello
2013/2/28 Dean Rasheed dean.a.rash...@gmail.com:
On 28 February 2013 11:25, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:
Umm. sorry,
If you have no problem with this, I'll send this to committer.
I just found that this patch already has a revewer. I've seen
only Status field in patch list..
Should I leave this to you, Dean?
Sorry, I've been meaning to review this properly for some time, but
I've been swamped with other work, so I'm happy for you to take over.
My overall impression is that the patch is in good shape, and provides
valuable new functionality, and it is probably close to being ready
for committer.
I think that the only other behavioural glitch I spotted was that it
fails to catch one overflow case, which should generate an out of
ranger error:
select format('|%*s|', -2147483648, 'foo');
Result: |foo|
because -(-2147483648) = 0 in signed 32-bit integers.
fixed - next other overflow check added
Regards
Pavel
Apart from that, I didn't find any problems during my testing.
Thanks for your review.
Regards,
Dean
format-width-20130301.patch
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello, Could you let me review this patch?
* merged Dean's doc
* allow NULL as width
I understand that this patch aims pure expansion of format's
current behavior and to mimic the printf in SUS glibc (*1).
(*1) http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html
This patch seems to preserve the behaviors of current
implement. And also succeeds in mimicking almost of SUS without
very subtle difference.
Attached is the new patch which I've edited following the
comments below and some fixed of typos, and added a few regtests.
If you have no problem with this, I'll send this to committer.
What do you think of this?
My comments are below,
==
Following is a comment about the behavior.
- The minus('-') is a flag, not a sign nor a operator. So this
seems permitted to appear more than one time. For example,
printf(%---10s, hoge) yields the output
hoge__ safely. This is consistent with the behavior
when negative value is supplied to '-*' conversion.
Followings are some comments about coding,
in text_format_parse_digits,
- is_valid seems to be the primary return value so returning
this as function's return value should make the caller more
simple.
- Although the compiler should deal properly with that, I don't
think it proper to use the memory pointed by function
parameters as local working storage. *inum and *is_valid in
the while loop should be replaced with local variables and
set them after the values are settled.
for TEXT_FORMAT_NEXT_CHAR,
- This macro name sounds somewhat confusing and this could be
used also in text_format_parse_digits. I propose
FORWARD_PARSE_POINT instead. Also I removed end_ptr from
macro parameters but I'm not sure of the pertinence of that.
for text_format_parse_format:
- Using start_ptr as a working pointer makes the name
inappropriate.
- Out parameters seems somewhat redundant. indirect_width and
indirect_width_parameter could be merged using 0 to indicate
unnumbered.
for text_format:
- maximum number of function argument limited to FUNC_MAX_ARGS
(100), so no need to care of wrap around of argument index, I
suppose.
- Something seems confusing at the lines follow
| /* Not enough arguments? Deduct 1 to avoid counting format string. */
| if (arg nargs - 1)
This expression does not have so special meaning. The maximum
index in an zero-based array should not be equal to or larger
than the number of the elements of it. If that's not your
intent, some rewrite would be needed..
- Only int4 is directly read for width value in the latest
patch, but int2 can also be directly readable and it should
be needed.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
format-width-20130228.patch.bz2
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Umm. sorry, If you have no problem with this, I'll send this to committer. I just found that this patch already has a revewer. I've seen only Status field in patch list.. Should I leave this to you, Dean? -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello
I have no objections,
Thank you for update
Regards
Pavel
2013/2/28 Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp:
Hello, Could you let me review this patch?
* merged Dean's doc
* allow NULL as width
I understand that this patch aims pure expansion of format's
current behavior and to mimic the printf in SUS glibc (*1).
(*1) http://pubs.opengroup.org/onlinepubs/009695399/functions/printf.html
This patch seems to preserve the behaviors of current
implement. And also succeeds in mimicking almost of SUS without
very subtle difference.
Attached is the new patch which I've edited following the
comments below and some fixed of typos, and added a few regtests.
If you have no problem with this, I'll send this to committer.
What do you think of this?
My comments are below,
==
Following is a comment about the behavior.
- The minus('-') is a flag, not a sign nor a operator. So this
seems permitted to appear more than one time. For example,
printf(%---10s, hoge) yields the output
hoge__ safely. This is consistent with the behavior
when negative value is supplied to '-*' conversion.
Followings are some comments about coding,
in text_format_parse_digits,
- is_valid seems to be the primary return value so returning
this as function's return value should make the caller more
simple.
- Although the compiler should deal properly with that, I don't
think it proper to use the memory pointed by function
parameters as local working storage. *inum and *is_valid in
the while loop should be replaced with local variables and
set them after the values are settled.
for TEXT_FORMAT_NEXT_CHAR,
- This macro name sounds somewhat confusing and this could be
used also in text_format_parse_digits. I propose
FORWARD_PARSE_POINT instead. Also I removed end_ptr from
macro parameters but I'm not sure of the pertinence of that.
for text_format_parse_format:
- Using start_ptr as a working pointer makes the name
inappropriate.
- Out parameters seems somewhat redundant. indirect_width and
indirect_width_parameter could be merged using 0 to indicate
unnumbered.
for text_format:
- maximum number of function argument limited to FUNC_MAX_ARGS
(100), so no need to care of wrap around of argument index, I
suppose.
- Something seems confusing at the lines follow
| /* Not enough arguments? Deduct 1 to avoid counting format string. */
| if (arg nargs - 1)
This expression does not have so special meaning. The maximum
index in an zero-based array should not be equal to or larger
than the number of the elements of it. If that's not your
intent, some rewrite would be needed..
- Only int4 is directly read for width value in the latest
patch, but int2 can also be directly readable and it should
be needed.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
On 28 February 2013 11:25, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:
Umm. sorry,
If you have no problem with this, I'll send this to committer.
I just found that this patch already has a revewer. I've seen
only Status field in patch list..
Should I leave this to you, Dean?
Sorry, I've been meaning to review this properly for some time, but
I've been swamped with other work, so I'm happy for you to take over.
My overall impression is that the patch is in good shape, and provides
valuable new functionality, and it is probably close to being ready
for committer.
I think that the only other behavioural glitch I spotted was that it
fails to catch one overflow case, which should generate an out of
ranger error:
select format('|%*s|', -2147483648, 'foo');
Result: |foo|
because -(-2147483648) = 0 in signed 32-bit integers.
Apart from that, I didn't find any problems during my testing.
Thanks for your review.
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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello
2013/2/13 Dean Rasheed dean.a.rash...@gmail.com:
On 11 February 2013 14:29, Pavel Stehule pavel.steh...@gmail.com wrote:
Hello
updated patch
* merged Dean's doc
* allow NULL as width
Hi,
I have not had time to look at this properly, but it doesn't look as
though you have fixed the other problem I mentioned up-thread, with %s
for NULL values:
SELECT format('|%s|', NULL);
Result: ||
SELECT format('|%5s|', NULL);
Result: ||
In the second case, I think it should produce | |.
fixed
Regards
Pavel Stehule
Regards,
Dean
format-width-20130213.patch
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
On 11 February 2013 14:29, Pavel Stehule pavel.steh...@gmail.com wrote:
Hello
updated patch
* merged Dean's doc
* allow NULL as width
Hi,
I have not had time to look at this properly, but it doesn't look as
though you have fixed the other problem I mentioned up-thread, with %s
for NULL values:
SELECT format('|%s|', NULL);
Result: ||
SELECT format('|%5s|', NULL);
Result: ||
In the second case, I think it should produce | |.
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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
On 10 February 2013 12:37, Pavel Stehule pavel.steh...@gmail.com Here is my first draft. I've also attached the generated HTML page, because it's not so easy to read an SGML patch. nice I have only one point - I am think, so format function should be in table 9-6 - some small text with reference to special section. It is already there in table 9-6, referring to the new section. Here is a minor update though -- I changed the name of the first optional argument from str to formatarg, since they are no longer necessarily strings. Regards, Dean format-width.doc.v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello updated patch * merged Dean's doc * allow NULL as width Regards Pavel 2013/2/11 Dean Rasheed dean.a.rash...@gmail.com: On 10 February 2013 12:37, Pavel Stehule pavel.steh...@gmail.com Here is my first draft. I've also attached the generated HTML page, because it's not so easy to read an SGML patch. nice I have only one point - I am think, so format function should be in table 9-6 - some small text with reference to special section. It is already there in table 9-6, referring to the new section. Here is a minor update though -- I changed the name of the first optional argument from str to formatarg, since they are no longer necessarily strings. Regards, Dean format-width-20130211.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
2013/2/10 Dean Rasheed dean.a.rash...@gmail.com:
On 9 February 2013 18:30, Pavel Stehule pavel.steh...@gmail.com wrote:
There are a new question
what should be result of
format(%2$*1$s, NULL, hello)
???
My first thought is that a NULL width should be treated the same as no
width at all (equivalent to width=0), rather than raising an
exception.
minor update - fix align NULL for %L
You need to do the same for a NULL value with %s, which currently
produces an empty string regardless of the width.
have others same opinion? Usually I don't like hide NULLs, but this is
corner case (and specific function) and I have not strong opinion on
this issue.
One use case for this might be something like
SELECT format('%*s', minimum_width, value) FROM some_table;
Throwing an exception when minimum_width is NULL doesn't seem
particularly useful. Intuitively, it just means that row has no
minimum width, so I think we should allow it.
I think the case where the value is NULL is much more clear-cut.
format('%s') produces '' (empty string). So format('%3s') should
produce ' '.
ok - in this case I can accept NULL as ignore width
The documentation also needs to be updated. I'm thinking perhaps
format() should now have its own separate sub-section in the manual,
rather than trying to cram it's docs into a single table row. I can
help with the docs if you like.
please, if you can, write it. I am sure, so you do it significantly
better than me.
Here is my first draft. I've also attached the generated HTML page,
because it's not so easy to read an SGML patch.
nice
I have only one point - I am think, so format function should be in
table 9-6 - some small text with reference to special section.
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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
2013/1/31 Pavel Stehule pavel.steh...@gmail.com: I am sending rewritten code Nice. I think this will be very useful, and it looks like it now supports everything that printf() does for %s format specifiers, and it's good that %I and %L behave the same. Also the code is looking cleaner. It indirect width * and *n$ is supported. It needs little bit more code. There are a new question what should be result of format(%2$*1$s, NULL, hello) ??? My first thought is that a NULL width should be treated the same as no width at all (equivalent to width=0), rather than raising an exception. minor update - fix align NULL for %L You need to do the same for a NULL value with %s, which currently produces an empty string regardless of the width. The documentation also needs to be updated. I'm thinking perhaps format() should now have its own separate sub-section in the manual, rather than trying to cram it's docs into a single table row. I can help with the docs if you like. 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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
2013/2/9 Dean Rasheed dean.a.rash...@gmail.com: 2013/1/31 Pavel Stehule pavel.steh...@gmail.com: I am sending rewritten code Nice. I think this will be very useful, and it looks like it now supports everything that printf() does for %s format specifiers, and it's good that %I and %L behave the same. Also the code is looking cleaner. It indirect width * and *n$ is supported. It needs little bit more code. There are a new question what should be result of format(%2$*1$s, NULL, hello) ??? My first thought is that a NULL width should be treated the same as no width at all (equivalent to width=0), rather than raising an exception. minor update - fix align NULL for %L You need to do the same for a NULL value with %s, which currently produces an empty string regardless of the width. have others same opinion? Usually I don't like hide NULLs, but this is corner case (and specific function) and I have not strong opinion on this issue. The documentation also needs to be updated. I'm thinking perhaps format() should now have its own separate sub-section in the manual, rather than trying to cram it's docs into a single table row. I can help with the docs if you like. please, if you can, write it. I am sure, so you do it significantly better than me. Thank you 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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello 2013/1/29 Dean Rasheed dean.a.rash...@gmail.com: On 29 January 2013 08:19, Dean Rasheed dean.a.rash...@gmail.com wrote: * The width field is optional, even if the '-' flag is specified. So '%-s' is perfectly legal and should be interpreted as '%s'. The current implementation treats it as a width of 0, which is wrong. Oh, but of course a width of 0 is the same as no width at all, so the current code is correct after all. That's what happens if I try to write emails before I've had my caffeine :-) I think my other points remain valid though. It would still be neater to parse the flags separately from the width field, and then all literal numbers that appear in the format should be positive. I am sending rewritten code It indirect width * and *n$ is supported. It needs little bit more code. There are a new question what should be result of format(%2$*1$s, NULL, hello) ??? raise exception now, but I am able to modify to some agreement Regards Pavel Regards, Dean format_width_20130131.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello minor update - fix align NULL for %L Regards Pavel 2013/1/31 Pavel Stehule pavel.steh...@gmail.com: Hello 2013/1/29 Dean Rasheed dean.a.rash...@gmail.com: On 29 January 2013 08:19, Dean Rasheed dean.a.rash...@gmail.com wrote: * The width field is optional, even if the '-' flag is specified. So '%-s' is perfectly legal and should be interpreted as '%s'. The current implementation treats it as a width of 0, which is wrong. Oh, but of course a width of 0 is the same as no width at all, so the current code is correct after all. That's what happens if I try to write emails before I've had my caffeine :-) I think my other points remain valid though. It would still be neater to parse the flags separately from the width field, and then all literal numbers that appear in the format should be positive. I am sending rewritten code It indirect width * and *n$ is supported. It needs little bit more code. There are a new question what should be result of format(%2$*1$s, NULL, hello) ??? raise exception now, but I am able to modify to some agreement Regards Pavel Regards, Dean format_width_20130201.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
On 28 January 2013 20:32, Dean Rasheed dean.a.rash...@gmail.com wrote: In general a format specifier looks like: %[parameter][flags][width][.precision][length]type This highlights another problem with the current implementation --- the '-' flag and the width field need to be parsed separately. So '%-3s' should be parsed as a '-' flag followed by a width of 3, not as a width of -3, which is then interpreted as left-aligned. This might seem like nitpicking, but actually it is important: * In the future we might support more flags, and they can be specified in any order, so the '-' flag won't necessarily come immediately before the width. * The width field is optional, even if the '-' flag is specified. So '%-s' is perfectly legal and should be interpreted as '%s'. The current implementation treats it as a width of 0, which is wrong. * The width field might not be a number, it might be something like * or *3$. Note that the SUS allows a negative width to be passed in as a function argument using this syntax, in which case it should be treated as if the '-' flag were specified. 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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
On 29 January 2013 08:19, Dean Rasheed dean.a.rash...@gmail.com wrote: * The width field is optional, even if the '-' flag is specified. So '%-s' is perfectly legal and should be interpreted as '%s'. The current implementation treats it as a width of 0, which is wrong. Oh, but of course a width of 0 is the same as no width at all, so the current code is correct after all. That's what happens if I try to write emails before I've had my caffeine :-) I think my other points remain valid though. It would still be neater to parse the flags separately from the width field, and then all literal numbers that appear in the format should be positive. 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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
2013/1/28 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: 2013/1/28 Dean Rasheed dean.a.rash...@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 disagree - but I have not a arguments. I am thinking so current implementation is wrong, and now is last time when we can to fix it - format() function is not too old and there is relative chance to minimal impact to users. I didn't propose removing this functionality, but fixing via more logical independent counter for ordered arguments. Dependency on previous positional argument is unpractical and unclean. I am not satisfied so it is undefined and then it is ok. Regards Pavel 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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
2013/1/28 Tom Lane t...@sss.pgh.pa.us: Dean Rasheed dean.a.rash...@gmail.com writes: On 28 January 2013 20:40, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/1/28 Dean Rasheed dean.a.rash...@gmail.com: 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. Left/right alignment and padding in printf() apply to all types, after the data value is converted to a string. Why shouldn't that same principle apply to %I and %L? I agree with Dean --- it would be very strange for these features not to apply to all conversion specifiers (excepting %% of course, which isn't really a conversion specifier but an escaping hack). ok - I have no problem with it - after some thinking - just remove one check. Regards Pavel 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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
2013/1/29 Dean Rasheed dean.a.rash...@gmail.com: On 28 January 2013 20:32, Dean Rasheed dean.a.rash...@gmail.com wrote: In general a format specifier looks like: %[parameter][flags][width][.precision][length]type This highlights another problem with the current implementation --- the '-' flag and the width field need to be parsed separately. So '%-3s' should be parsed as a '-' flag followed by a width of 3, not as a width of -3, which is then interpreted as left-aligned. This might seem like nitpicking, but actually it is important: * In the future we might support more flags, and they can be specified in any order, so the '-' flag won't necessarily come immediately before the width. * The width field is optional, even if the '-' flag is specified. So '%-s' is perfectly legal and should be interpreted as '%s'. The current implementation treats it as a width of 0, which is wrong. * The width field might not be a number, it might be something like * or *3$. Note that the SUS allows a negative width to be passed in as a function argument using this syntax, in which case it should be treated as if the '-' flag were specified. A possibility to specify width as * can be implemented in future. The format() function was not designed to be fully compatible with SUS - it is simplified subset with pg enhancing. There was a talks about integration to_char() formats to format() and we didn't block it - and it was reason why I proposed and pushed name format and not printf, because there can be little bit different purposes than generic printf function. 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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
On 26 January 2013 10:58, Pavel Stehule pavel.steh...@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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Hello
2013/1/28 Dean Rasheed dean.a.rash...@gmail.com:
On 26 January 2013 10:58, Pavel Stehule pavel.steh...@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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Pavel Stehule pavel.steh...@gmail.com writes: 2013/1/28 Dean Rasheed dean.a.rash...@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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
* Tom Lane (t...@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 signature.asc Description: Digital signature
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
On 28 January 2013 17:32, Stephen Frost sfr...@snowman.net wrote: * Tom Lane (t...@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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
2013/1/28 Dean Rasheed dean.a.rash...@gmail.com: On 28 January 2013 17:32, Stephen Frost sfr...@snowman.net wrote: * Tom Lane (t...@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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
On 28 January 2013 20:40, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/1/28 Dean Rasheed dean.a.rash...@gmail.com: On 28 January 2013 17:32, Stephen Frost sfr...@snowman.net wrote: * Tom Lane (t...@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. Left/right alignment and padding in printf() apply to all types, after the data value is converted to a string. Why shouldn't that same principle apply to %I and %L? The obvious use-case is for producing tabular output of data with columns neatly aligned. If we don't support %I and %L then any alignment of columns to the right is lost. 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
Re: [HACKERS] Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used
Dean Rasheed dean.a.rash...@gmail.com writes: On 28 January 2013 20:40, Pavel Stehule pavel.steh...@gmail.com wrote: 2013/1/28 Dean Rasheed dean.a.rash...@gmail.com: 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. Left/right alignment and padding in printf() apply to all types, after the data value is converted to a string. Why shouldn't that same principle apply to %I and %L? I agree with Dean --- it would be very strange for these features not to apply to all conversion specifiers (excepting %% of course, which isn't really a conversion specifier but an escaping hack). 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
