Re: pg_execute_from_file review

Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com> writes:
> I have some comments and questions about
> pg_execute_from_file.v5.patch.

I believe v6 fixes it all, please find it attached.

> ==== Source code ====
> * OID=3627 is used by another function. Don't you expect 3927?
>
> * There is a compiler warning:
> genfile.c: In function ‘pg_execute_from_file_with_placeholders’:
> genfile.c:427: warning: unused variable ‘query_string’

Both fixes are in, sorry again.

> * I'd like to ask native speakers whether "from" is needed in names
> of "pg_execute_from_file" and "pg_execute_from_query_string".

The function name now is pg_execute_sql_file(), per comments from
Andrew, Joshua, Robert and Tom, all qualified native English speakers,
among other qualities :)

> ==== Design and Implementation ====
> * pg_execute_from_file() can execute any files even if they are not
> in $PGDATA. OTOH, we restrict pg_read_file() to read such files.
> What will be our policy? Note that the contents of file might be
> logged or returned to the client on errors.
>
> * Do we have any reasons to have pg_execute_from_file separated from
> pg_read_file ? If we allow pg_read_file() to read files in $PGSHARE,
> pg_execute_from_file could be replaced with "EXECUTE pg_read_file()".
> (Note that pg_execute_from_file is implemented to do so even now.)

Thinking some more about it, there's still a reason to maintain them
separated: the API ain't the same, we're not proposing to read a sql
script file chunk after chunk, nor do we want users to have to check for
the file size before being able to call the function.

A problem with pg_read_file() as it stands is that it's returning text
rather than bytea, too, and if we choose to fix that rather than adding
some new functions, we will want to avoid having to separate the two
functions again.

> * I hope pg_execute_from_file (and pg_read_file) had an option
> to specify an character encoding of the file. Especially, SJIS
> is still used widely, but it is not a valid server encoding.

So, what about client_encoding here, again?
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support