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 <function>quote_ident</function> and <function>quote_literal</function> functions, because this function doesn't try to coerce parameters to <type>text</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 <function>quote_nullable</function> function, as the latter coerces its argument to <type>text</type> while <function>format</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 <function>format</function> function (see <xref linkend="functions-string">): <programlisting> EXECUTE format('UPDATE tbl SET %I = %L WHERE key = %L', colname, newvalue, keyvalue); </programlisting> The <function>format</function> format can be used together with the <literal>USING</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 <literal>newvalue</literal> and <literal>keyvalue</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