The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, failed
Spec compliant:           tested, failed
Documentation:            not tested

Contents & Purpose

This patch adds a new type of psql variables: file references, that can be
set using command \setfileref. The value of the named variable is the path
to the referenced file. It allows simple inserting of large data without
necessity of manual escaping or using LO api. Use:

        postgres=# create table test (col1 bytea);
        postgres=# \setfileref a ~/avatar.gif
        postgres=# insert into test values(:a);

Content of file is returned as bytea, the text output can be used when
escaping is not required, in this case use convert_from() to obtain a
text output.

        postgres=# create table test (col1 text);
        postgres=# \setfileref a ~/README.txt
        postgres=# insert into test values(convert_from(:a, 'utf8'));

The content of file reference variables is not persistent in memory.

List of file referenced variable can be listed using \set

        postgres=# \set
        b = ^'~/README.txt'

Empty file outputs an empty bytea (\x).

Initial Run

The patch applies cleanly to HEAD and doesn't break anything, at least the
regression tests all pass successfully.

Feedback on testing

I see several problems.

1) Error reading referenced file returns the system error and a syntax error
on variable:

        postgres=# \setfileref a /etc/sudoers
        postgres=# insert into test values (:a);
        /etc/sudoers: Permission denied
        ERROR:  syntax error at or near ":"
        LINE 1: insert into t1 values (:a);

2) Trying to load a file with size upper than the 1GB limit reports the 
error (size 2254110720 bytes):

        postgres=# \setfileref b /home/VMs/ISOs/sol-10-u11-ga-x86-dvd.iso
        postgres=# insert into test values (:b);
        INSERT 0 1
        postgres=# ANALYZE test;
        postgres=# SELECT relname, relkind, relpages, 
pg_size_pretty(relpages::bigint*8192) FROM pg_class WHERE relname='test';
         relname | relkind | relpages | pg_size_pretty 
         test    | r       |        1 | 8192 bytes
        (1 row)

        postgres=# select * from test;
        (1 row)

This should not insert an empty bytea but might raise an error instead.

Trying to load smaller file but with bytea escaping total size upper than
the 1GB limit (size 666894336 bytes) reports:

        postgres=# \setfileref a /var/ISOs/CentOS-7-x86_64-Minimal-1503-01.iso
        postgres=# insert into t1 values (1, :a, 'tr');
        ERROR:  out of memory
        DETAIL:  Cannot enlarge string buffer containing 0 bytes by 1333788688 
more bytes.
        Time: 1428.657 ms (00:01.429)

This raise an error as bytea escaping increase content size which is the 
behavior expected.

3) The path doesn't not allow the use of pipe to system command, which is a 
behavior, but it is quite easy to perform a DOS by using special files like:

        postgres=# \setfileref b /dev/random                                    
        postgres=# insert into t1 (:b);.

this will never end until we kill the psql session. I think we must also prevent
non regular files to be referenced using S_ISREG.

I think all these three errors can be caught and prevented at variable 
declaration using some tests on files like:

        is readable?
        is a regular file?
        is small size enough?

to report an error directly at variable file reference setting.

4) An other problem is that like this this patch will allow anyone to upload 
into a
column the content of any system file that can be read by postgres system user
and then allow non system user to read its content. For example, connected as
a basic PostgreSQL only user:

        testdb=> select current_user;
        (1 row)
        testdb=> \setfileref b /etc/passwd
        testdb=> insert into test values (:b);
        INSERT 0 1

then to read the file:

        testdb=> select convert_from(col1, 'utf8') from t1; 

Maybe the referenced files must be limited to some part of the filesystem
or the feature limited to superuser only.

5) There is no documentation at all on this feature. Here a proposal
for inclusion in doc/src/sgml/ref/psql-ref.sgml 

        \setfileref name value
        Sets the internal variable name as a reference to the file path
        set as value. To unset a variable, use the \unset command.

        File references are shown as file path prefixed with the ^ character
        when using the \set command alone.

        Valid variable names can contain characters, digits, and underscores.
        See the section Variables below for details. Variable names are

        More detail here about file size, file privilege, etc following
        what will be decided.

6) I would also like to see \setfileref display all file references variables
defined instead of message "missing required argument". Just like \set and
\pset alone are working. \set can still show the file references variables, 
not a problem for me.

The new status of this patch is: Waiting on Author

Sent via pgsql-hackers mailing list (
To make changes to your subscription:

Reply via email to