From: | Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com> |
---|---|
To: | Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> |
Cc: | Joshua Tolley <eggyknap(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: pg_execute_from_file review |
Date: | 2010-11-29 06:20:32 |
Message-ID: | AANLkTi=j0Fz-7CKnk=Pt6cxsvZ=DF6u=1veWmhWS9EA2@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Nov 26, 2010 at 06:24, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
> Thanks for your review. Please find attached a revised patch where I've
> changed the internals of the function so that it's split in two and that
> the opr_sanity check passes, per comments from David Wheeler and Tom Lane.
I have some comments and questions about pg_execute_from_file.v5.patch.
==== 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’
* I'd like to ask native speakers whether "from" is needed in names
of "pg_execute_from_file" and "pg_execute_from_query_string".
==== 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.)
* 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.
--
Itagaki Takahiro
From | Date | Subject | |
---|---|---|---|
Next Message | David Fetter | 2010-11-29 06:58:40 | Re: Patch to add a primary key using an existing index |
Previous Message | Noah Misch | 2010-11-29 06:10:38 | On-the-fly index tuple deletion vs. hot_standby |