Re: [HACKERS] Fwd: patch: format function - fixed oid

2010-11-21 Thread Robert Haas 
On Nov 21, 2010, at 1:09 AM, Pavel Stehule pavel.steh...@gmail.com wrote:
 
 OK, I've committed this, after a fairly heavy rewrite.
 
 thank you very much

Ah, nuts. I forgot to bump catversion.

...Robert

Re: [HACKERS] Fwd: patch: format function - fixed oid

2010-11-20 Thread Pavel Stehule 
Hello

2010/11/20 Jeff Janes jeff.ja...@gmail.com:
 On Thu, Nov 18, 2010 at 11:54 PM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 -- Forwarded message --
 From: Pavel Stehule pavel.steh...@gmail.com
 Date: 2010/11/18
 Subject: Re: patch: format function, next generation
 To: Jeff Janes jeff.ja...@gmail.com
 Kopie: pgsql-hackers-ow...@postgresql.org


 Hello

 somebody takes my oid :)

 updated patch is in attachment

 Regards

 Pavel Stehule


 Dear Pavel and Hackers,

 I've reviewed this patch.  It applied, makes, and passes make check.
 It has added regression tests that seem appropriate.  I think the
 feature added matches the consensus that emerged from the very long
 email discussion.  The C code seems fine (to my meager abilities to
 judge that).

 But I think the documentation does need some work.  From func.sgml:


         This functions can be used to create a formated string or
 message. There are allowed
         three types of tags: %s as string, %I as SQL identifiers and
 %L as SQL literals. Attention:
         result for %I and %L must not be same as result of
 functionquote_ident/function and
         functionquote_literal/function functions, because this
 function doesn't try to coerce
         parameters to typetext/type type and directly use a
 type's output functions. The placeholder
         can be related to some explicit parameter with using a
 optional n$ specification inside format.

 Should we make it explicit that this is inspired by C's sprintf?  Do
 we want to call them tags?  This is introducing what seems to be a
 new word to describe what are usually (I think) called conversion
 specifiers.


I am not native speaker, so I invite any correction in documentation -
anything what I wrote is more/less just skelet for somebody, whu can
speak better than me. I am not against to note - so this function is
similar to sprintf function, but then should be specified - so this
function is different - designed to request PL languages and
environment. I have not a problem with replacing tags by conversion
or positional specifiers. Somewhere is used term placeholder??

 Must not be the same  should be Might not be the same.   However,
 it does not appear that quote_ident is willing to use coercion at all,
 and the %L behavior is more comparable to quote_nullable.

 Maybe:

 This function can be used to create a formatted string suitable for
 use as dynamic
 SQL or as a message.  There are three types of conversion specifiers:
 %s for literal strings, %I
 for SQL identifiers, and %L for SQL literals.  Note that the results
 of the %L conversion
 might not be the same as the results of the
 functionquote_nullable/function function, as
 the latter coerces its argument to typetext/type while
 functionformat/function
 uses a type's output function.  A conversion can reference an explicit
 parameter position
 by using an optional n$ in the format specification.


I am for it

 Does type's output function need to cross-reference someplace?
 coercion is described elsewhere in this section of docs, but output
 functions are not.


probably output functions are described in some hacker parts
http://www.postgresql.org/docs/9.0/interactive/xtypes.html


 And for the changes to plpgsql.sgml, I would propose:

    para
     Building a string for dynamic SQL statement can be simplified
     by using the functionformat/function function (see xref
     linkend=functions-string):
 programlisting
 EXECUTE format('UPDATE tbl SET %I = %L WHERE key = %L', colname,
 newvalue, keyvalue);
 /programlisting
    The functionformat/function format can be used together with
     the literalUSING/literal clause:
 programlisting
 EXECUTE format('UPDATE tbl SET %I = $1 WHERE key = $2', colname)
   USING newvalue, keyvalue;
 /programlisting
     This form is more efficient because the parameters
     literalnewvalue/literal and literalkeyvalue/literal are
 not converted to text.
    /para


+1


 These are mostly grammatical changes, but with the last three lines I
 may have missed the meaning of what you originally intended--I'm not
 sure on that.


I think so you are a correct. Who will a submit this final version? You or me?

Thank you very much

Regards

Pavel Stehule



 Thanks,

 Jeff


-- 
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] Fwd: patch: format function - fixed oid

2010-11-20 Thread Robert Haas 
On Sat, Nov 20, 2010 at 2:59 AM, Pavel Stehule pavel.steh...@gmail.com wrote:
 I think so you are a correct. Who will a submit this final version? You or me?

I've got it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] Fwd: patch: format function - fixed oid

2010-11-20 Thread Pavel Stehule 
2010/11/20 Robert Haas robertmh...@gmail.com:
 On Sat, Nov 20, 2010 at 2:59 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 I think so you are a correct. Who will a submit this final version? You or 
 me?

 I've got it.


Thank you

nice a day

Pavel

 --
 Robert Haas
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company


-- 
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] Fwd: patch: format function - fixed oid

2010-11-20 Thread Robert Haas 
On Sat, Nov 20, 2010 at 7:29 AM, Pavel Stehule pavel.steh...@gmail.com wrote:
 2010/11/20 Robert Haas robertmh...@gmail.com:
 On Sat, Nov 20, 2010 at 2:59 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 I think so you are a correct. Who will a submit this final version? You or 
 me?

 I've got it.


 Thank you

 nice a day

OK, I've committed this, after a fairly heavy rewrite.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] Fwd: patch: format function - fixed oid

2010-11-20 Thread Pavel Stehule 
2010/11/21 Robert Haas robertmh...@gmail.com:
 On Sat, Nov 20, 2010 at 7:29 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 2010/11/20 Robert Haas robertmh...@gmail.com:
 On Sat, Nov 20, 2010 at 2:59 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 I think so you are a correct. Who will a submit this final version? You or 
 me?

 I've got it.


 Thank you

 nice a day

 OK, I've committed this, after a fairly heavy rewrite.

thank you very much

regards

Pavel Stehule

 --
 Robert Haas
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company


-- 
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] Fwd: patch: format function - fixed oid

2010-11-19 Thread Jeff Janes 
On Thu, Nov 18, 2010 at 11:54 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 -- Forwarded message --
 From: Pavel Stehule pavel.steh...@gmail.com
 Date: 2010/11/18
 Subject: Re: patch: format function, next generation
 To: Jeff Janes jeff.ja...@gmail.com
 Kopie: pgsql-hackers-ow...@postgresql.org


 Hello

 somebody takes my oid :)

 updated patch is in attachment

 Regards

 Pavel Stehule


Dear Pavel and Hackers,

I've reviewed this patch.  It applied, makes, and passes make check.
It has added regression tests that seem appropriate.  I think the
feature added matches the consensus that emerged from the very long
email discussion.  The C code seems fine (to my meager abilities to
judge that).

But I think the documentation does need some work.  From func.sgml:


 This functions can be used to create a formated string or
message. There are allowed
 three types of tags: %s as string, %I as SQL identifiers and
%L as SQL literals. Attention:
 result for %I and %L must not be same as result of
functionquote_ident/function and
 functionquote_literal/function functions, because this
function doesn't try to coerce
 parameters to typetext/type type and directly use a
type's output functions. The placeholder
 can be related to some explicit parameter with using a
optional n$ specification inside format.

Should we make it explicit that this is inspired by C's sprintf?  Do
we want to call them tags?  This is introducing what seems to be a
new word to describe what are usually (I think) called conversion
specifiers.

Must not be the same  should be Might not be the same.   However,
it does not appear that quote_ident is willing to use coercion at all,
and the %L behavior is more comparable to quote_nullable.

Maybe:

This function can be used to create a formatted string suitable for
use as dynamic
SQL or as a message.  There are three types of conversion specifiers:
%s for literal strings, %I
for SQL identifiers, and %L for SQL literals.  Note that the results
of the %L conversion
might not be the same as the results of the
functionquote_nullable/function function, as
the latter coerces its argument to typetext/type while
functionformat/function
uses a type's output function.  A conversion can reference an explicit
parameter position
by using an optional n$ in the format specification.

Does type's output function need to cross-reference someplace?
coercion is described elsewhere in this section of docs, but output
functions are not.


And for the changes to plpgsql.sgml, I would propose:

para
 Building a string for dynamic SQL statement can be simplified
 by using the functionformat/function function (see xref
 linkend=functions-string):
programlisting
EXECUTE format('UPDATE tbl SET %I = %L WHERE key = %L', colname,
newvalue, keyvalue);
/programlisting
The functionformat/function format can be used together with
 the literalUSING/literal clause:
programlisting
EXECUTE format('UPDATE tbl SET %I = $1 WHERE key = $2', colname)
   USING newvalue, keyvalue;
/programlisting
 This form is more efficient because the parameters
 literalnewvalue/literal and literalkeyvalue/literal are
not converted to text.
/para


These are mostly grammatical changes, but with the last three lines I
may have missed the meaning of what you originally intended--I'm not
sure on that.


Thanks,

Jeff

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

[HACKERS] Fwd: patch: format function - fixed oid

2010-11-18 Thread Pavel Stehule 
-- Forwarded message --
From: Pavel Stehule pavel.steh...@gmail.com
Date: 2010/11/18
Subject: Re: patch: format function, next generation
To: Jeff Janes jeff.ja...@gmail.com
Kopie: pgsql-hackers-ow...@postgresql.org


Hello

somebody takes my oid :)

updated patch is in attachment

Regards

Pavel Stehule

2010/11/18 Jeff Janes jeff.ja...@gmail.com:

 Hello

 I reworked a implementation of format function. This respects last 
 discussion:

 * support a positional placeholders - syntax %99$x,
 * support a tags: %s, I, L,
 * enhanced documentation,
 * enhanced reggress tests

 Regards

 Pavel Stehule

 Dear Pavel,

 Your patch no longer passes make check against git HEAD.

 It looks like the backend sync commit has claimed OID 3063

 creating directory
 /home/jjanes/postgres/git/src/test/regress/./tmp_check/data ... ok
 creating subdirectories ... ok
 selecting default max_connections ... 100
 selecting default shared_buffers ... 32MB
 creating configuration files ... ok
 creating template1 database in
 /home/jjanes/postgres/git/src/test/regress/./tmp_check/data/base/1 ...
 FATAL:  could not create unique index pg_proc_oid_index
 DETAIL:  Key (oid)=(3063) is duplicated.
 child process exited with exit code 1
 initdb: data directory
 /home/jjanes/postgres/git/src/test/regress/./tmp_check/data not
 removed at user's request

 Cheers,

 Jeff

*** ./doc/src/sgml/func.sgml.orig	2010-11-18 11:34:18.183045054 +0100
--- ./doc/src/sgml/func.sgml	2010-11-18 11:35:59.799401973 +0100
***
*** 1272,1277 
--- 1272,1280 
  primaryencode/primary
 /indexterm
 indexterm
+ primaryformat/primary
+/indexterm
+indexterm
  primaryinitcap/primary
 /indexterm
 indexterm
***
*** 1497,1502 
--- 1500,1524 
/row   
  
row
+entry
+ literalfunctionformat/function(parameterformatstr/parameter typetext/type 
+ [, parameterstr/parameter typeany/type [, ...] ])/literal
+/entry
+entrytypetext/type/entry
+entry
+  This functions can be used to create a formated string or message. There are allowed
+  three types of tags: %s as string, %I as SQL identifiers and %L as SQL literals. Attention:
+  result for %I and %L must not be same as result of functionquote_ident/function and 
+  functionquote_literal/function functions, because this function doesn't try to coerce 
+  parameters to typetext/type type and directly use a type's output functions. The placeholder
+  can be related to some explicit parameter with using a optional n$ specification inside format.
+  See also xref linkend=plpgsql-quote-literal-example.
+/entry
+entryliteralformat('Hello %s, %1$s', 'World')/literal/entry
+entryliteralHello World, World/literal/entry
+   /row   
+ 
+   row
 entryliteralfunctioninitcap(parameterstring/parameter)/function/literal/entry
 entrytypetext/type/entry
 entry
*** ./doc/src/sgml/plpgsql.sgml.orig	2010-11-18 11:34:35.608335452 +0100
--- ./doc/src/sgml/plpgsql.sgml	2010-11-18 11:35:59.817404340 +0100
***
*** 1250,1255 
--- 1250,1273 
   emphasismust/ use functionquote_literal/,
   functionquote_nullable/, or functionquote_ident/, as appropriate.
  /para
+ 
+ para
+  Building a string with dynamic SQL statement can be simplified 
+  with using a functionformat/function function (see xref
+  linkend=functions-string):
+ programlisting
+ EXECUTE format('UPDATE tbl SET %I = %L WHERE key = %L', colname, newvalue, keyvalue);
+ /programlisting
+  A use the functionformat/function format can be together with 
+  literalUSING/literal clause:
+ programlisting
+ EXECUTE format('UPDATE tbl SET %I = $1 WHERE key = $2', colname)
+USING newvalue, keyvalue;
+ /programlisting
+  This form is little bit more efective, because a parameters
+  literalnewvalue/literal and literalkeyvalue/literal must not be
+  converted to text.
+ /para
 /example
  
  para
*** ./src/backend/utils/adt/varlena.c.orig	2010-11-18 11:34:49.891212809 +0100
--- ./src/backend/utils/adt/varlena.c	2010-11-18 11:35:59.823405128 +0100
***
*** 3702,3704 
--- 3702,3939 
  
  	PG_RETURN_TEXT_P(result);
  }
+ 
+ typedef struct {
+ 	char	field_type;
+ 	int32	position;
+ 	bool	is_positional;
+ } placeholder_desc;
+ 
+ #define INVALID_PARAMETER_ERROR(d)	ereport(ERROR, \
+ 			(errcode(ERRCODE_INVALID_PARAMETER_VALUE), \
+ 			 errmsg(invalid formatting string for function format), \
+ 			 errdetail(d)))
+ 
+ static const char *
+ parse_format(placeholder_desc *pd, const char *start_ptr, 
+ 			const char *end_ptr, int nparams)
+ {
+ 	bool	is_valid = false;
+ 
+ 	pd-is_positional = false;
+ 
+ 	while (start_ptr = end_ptr)
+ 	{
+ 		switch (*start_ptr)
+ 		{
+ 			case 's':
+ pd-field_type = 's';
+ is_valid =