Re: pg_execute_from_file review

On Sun, Dec 5, 2010 at 6:01 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com> writes:
>> On Fri, Dec 3, 2010 at 18:02, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
>>> My understanding is that the variadic form shadows the other one in a
>>> way that it's now impossible to call it from SQL level. That's the
>>> reason why I did the (text, text, text, VARIADIC text) version before,
>>> but is it true?
>
>> The VARIADIC version doesn't hide the 3-args version. I tested the
>> behavior by printf-debug. The planner seems to think the non VARIADIC
>> version is the best-matched one when 3 arguments are passed.
>
> Why is there a variadic replace() in this patch at all?  It seems just
> about entirely unrelated to the stated purpose of the patch, as well
> as being of dubious usefulness.  When would it be superior to
>
>        replace(replace(orig, from1, to1), from2, to2), ...
>
> The implementation doesn't appear to offer any material speed
> improvement over nested calls of that sort, and I'm finding it hard to
> visualize when it would be more useful than nested calls.  The
> documentation is entirely inadequate as well.

An iterated replacement has different semantics from a simultaneous
replace - replacing N placeholders with values simultaneously means
you don't need to worry about the case where one of the replacement
strings contains something that looks like a placeholder. I actually
think a simultaneous replacement feature would be quite handy but I
make no comment on whether it belongs as part of this patch. The
discussion on this patch has been rather wide-ranging, and it's not
clear to me that there's really consensus on what this patch needs to
- or should - do.

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