Re: pg_execute_from_file review

On Mon, Dec 6, 2010 at 12:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Sun, Dec 5, 2010 at 6:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> 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), ...
>
>> 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.
>
> Good point, but what the patch implements is in fact iterated
> replacement ... or at least it looked that way in a quick once-over.

Oh.  Well, -1 from me for including that.

>> 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.
>
> My point is that the replacement stuff really really needs to be
> factored out of the string-execution stuff, precisely because the
> desired behavior is debatable.

+1 for committing the uncontroversial parts separately.

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