From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Jeff Janes <jeff(dot)janes(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Fwd: patch: format function - fixed oid |
Date: | 2010-11-20 07:59:16 |
Message-ID: | AANLkTi=knodZXCEBj95Kw6hbT4fWfk09J8BTSGFAafGy@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello
2010/11/20 Jeff Janes <jeff(dot)janes(at)gmail(dot)com>:
> On Thu, Nov 18, 2010 at 11:54 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> ---------- Forwarded message ----------
>> From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
>> Date: 2010/11/18
>> Subject: Re: patch: format function, next generation
>> To: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
>> Kopie: pgsql-hackers-owner(at)postgresql(dot)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
>
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2010-11-20 12:21:06 | Re: [PATCH] Custom code int(32|64) => text conversions out of performance reasons |
Previous Message | Itagaki Takahiro | 2010-11-20 04:54:32 | Re: UNNEST ... WITH ORDINALITY (AND POSSIBLY OTHER STUFF) |