On Thu, Dec 2, 2010 at 07:00, Dimitri Fontaine <dimi...@2ndquadrant.fr> wrote:
> Here's the result:
>
> dim=# \df pg_exe*|replace_*|*binary*
>                                     List of functions
>    Schema   |         Name          | Result data type |    Argument data 
> types    |  Type
> ------------+-----------------------+------------------+---------------------------+--------
>  pg_catalog | pg_execute_sql_file   | void             | text                 
>      | normal
>  pg_catalog | pg_execute_sql_file   | void             | text, name           
>      | normal
>  pg_catalog | pg_execute_sql_file   | void             | text, name, VARIADIC 
> text | normal
>  pg_catalog | pg_execute_sql_string | void             | text                 
>      | normal
>  pg_catalog | pg_execute_sql_string | void             | text, VARIADIC text  
>      | normal
>  pg_catalog | pg_read_binary_file   | bytea            | text, bigint, bigint 
>      | normal
>  pg_catalog | replace_placeholders  | text             | text, VARIADIC text  
>      | normal
> (7 rows)

Thanks, good job!

* pg_read_binary_file_internal() should return not only the contents
  as char * but also the length, because the file might contain 0x00.
  In addition, null-terminations for the contents buffer is useless.

* The 1st argument of pg_convert must be bytea rather than cstring in
  pg_convert_and_execute_sql_file(). I think you can fix both the bug
  and the above one if pg_read_binary_file_internal() returns bytea.

* pg_read_file() has stronger restrictions than pg_read_binary_file().
  (absolute path not allowed) and -1 length is not supported.
  We could fix pg_read_file() to behave as like as pg_read_binary_file().

* (It was my suggestion, but) Is it reasonable to use -1 length to read
  the while file? It might fit with C, but NULL might be better for SQL.

* The doc says pg_execute_sql_string() is restricted for superusers,
  but is not restricted actually. I think we don't have to.

* In docs, the example of replace_placeholders() is
  replace('abcdefabcdef', 'cd', 'XX', 'ef', 'YY').
  ~~~~~~~
  BTW, I think we can call it just "replace" because parser can handle
  them correctly even if we have both replace(text, text, text) and
  replace(text, VARIADIC text[]). We will need only one doc for them.
  Note that if we call replace() with 3 args, the non-VARIADIC version
  is called. So, there is no performance penalty.

* We might rename pg_convert_and_execute_sql_file() to
  pg_execute_sql_file_with_encoding() or so to have the same prefix.

-- 
Itagaki Takahiro

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to