Re: pg_execute_from_file review

On Tue, Nov 30, 2010 at 18:47, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
> Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com> writes:
>> There are no discussion yet for 1, but I think we need some restrictions
>
> Well, as a first level of restrictions, the function is superuser
> only. I understand and share your concerns, but as the main use for this
> function is in the extension's patch which is superuser only too,

I found superuser can read any files in the server via COPY FROM
and lo_import(). So, I think the restriction in pg_read_file() is
inconsistent and doesn't protect the server at all.

> Again, I'd like to see pg_read_binary_file() and it's easy to expose the
> other derivatives you're proposing here: the support code is already in
> my patch and is organised this way internally. Now, is there an
> agreement that all those new SQL functions this should be in the
> pg_execute_from_file patch? If so, I'll prepare v7 with that.

My suggestion is to introduce pg_read_binary_file() function that can
read any files in the server, and make CREATE EXTENSION to use the
function. Of course, pg_execute_[sql|from]_file() can simplify queries
in some degree, but I think it has too many jobs -- reading a file,
(converting the encoding), replacing some texts in it, and executing
the sql. If we have pg_read_binary_file(), you could implement
CREATE EXTENSION like as below using SPI or nested function calls:

$sql := replace(
convert_from(
pg_read_binary_file($path, 0, -1),
$encoding),
'@extschema@', $schema));
EXECUTE $sql;

You seem to want to replace one variable @extschema(at)(dot) So, you don't
need replace(VARIADIC) function. The patch will be a bit simpler.

--
Itagaki Takahiro