Re: pg_execute_from_file, patch v10

Tom Lane <tgl@sss.pgh.pa.us> writes:
> Robert Haas <robertmhaas@gmail.com> writes:
>> So there are really four changes in here, right?
>
>> 1. Relax pg_read_file() to be able to read any files.
>> 2. pg_read_binary_file()
>> 3. pg_execute_sql_string()/file()
>> 4. ability to read a file in a given encoding (rather than the client encoding)
>
>> I think I agree that #1 doesn't open any security hole that doesn't
>> exist already.
>
> That function would never have been accepted into core at all without a
> locked-down range of how much of the filesystem it would let you get at.

Ok. Previously pg_read_file() only allows absolute file names that point
into DataDir or into Log_directory. It used not to work in the first
versions of the extension's patch, but with the current code, the check
passes on a development install here: extension.c is only giving
genfile.c absolute file names.

Please note that debian will default to have DataDir in a different
place than the sharepath:
 http://packages.debian.org/sid/amd64/postgresql-contrib-9.0/filelist
 PGDATA:    /var/lib/postgresql/9.1/main  sharepath: /usr/share/postgresql/9.1/contrib libdir:
/usr/lib/postgresql/9.1/lib

So I'm not sure how if it will play nice with such installs, or if
there's already some genfile.c patching on debian.

>> I think #2 might be a nice thing to have, but I'm not sure what it has
>> to do with extensions.
>
> Agreed.  There might be some use for #4 in connection with extensions,
> but I don't see that #2 is related.

Well, in fact, the extension's code is using either execute_sql_file()
or read_text_file_with_endoding() then @extschema@ replacement then
execute_sql_string(), all those functions called directly thanks to
#include "utils/genfile.h". No DirectFunctionCall'ing, we can easily
remove SQL callable forms.

So what we need is 2, 3 and 4 (because 4 builds on 2).

> BTW, it appears to me that pg_read_file expects server encoding not
> client encoding.  Minor detail only, but let's be clear what it is
> we're talking about.

Hence the refactoring in the patch. Ask Itagaki for details with funny
environments using some file encoding that does not exists in the server
yet ain't client_encoding and can't be. I didn't follow the use case in
details, but he was happy with the current way of doing things and not
with any previous one.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support