Re: Fwd: patch: format function - fixed oid

Hello

2010/11/20 Jeff Janes <jeff.janes@gmail.com>:
> On Thu, Nov 18, 2010 at 11:54 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> ---------- Forwarded message ----------
>> From: Pavel Stehule <pavel.stehule@gmail.com>
>> Date: 2010/11/18
>> Subject: Re: patch: format function, next generation
>> To: Jeff Janes <jeff.janes@gmail.com>
>> Kopie: pgsql-hackers-owner@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".
>

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
> <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.
>

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 <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>
>

+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
>