On Thu, Dec 2, 2010 at 20:00, Dimitri Fontaine <dimi...@2ndquadrant.fr> wrote: > Please find attached the v8 version of the patch, that fixes the following:
I fixed and cleanup some of codes in it; v9 patch attached. Please check my modifications, and set the status to "Ready to Committer" if you find no problems. I think documentation and code comments might need to be checked by native English speakers. > Well thinking about it, omitting the length parameter alltogether seems > like the more natural SQL level API for me, so I've made it happen: Good idea. I re-added negative lengths checks in pg_read_file functions; negative length is used internally, but not exposed as SQL functions. >> BTW, I think we can call it just "replace" > The same idea occured to me yesternight when reading through the patch > to send. It's now done in the way you can see above. The idea is not to > change the existing behavior at all, so I've not changed the > non-VARIADIC version of the function. You added replace(text, text, text, VARIADIC text), but I think replace(text, VARIADIC text) also works. If we have the short form, we can use it easily from execute functions with placeholders. Other changes: * Added some regression tests. * Int64GetDatum((int64) fst.st_size) was broken. * An error checks for "could not read file" didn't work. * Read file contents into bytea buffer directly to avoid memcpy. * Fixed bad usages of text and bytea values because they are not null-terminated. * I don't want to expose ArrayType to builtins.h. So I call replace_text_variadic() from execute functions. * pg_execute_sql_file(path, NULL) won't work because it's a STRICT function. It returns NULL with no works when at least one of the argument is NULL. BTW, we have many text from/to cstring conversions in the new codes. It would be not an item for now, but we would need to improve them if those functions are heavily used, Especially replace_text_variadic(). >> * We might rename pg_convert_and_execute_sql_file() to >> pg_execute_sql_file_with_encoding() or so to have the same prefix. > > Well, I think I prefer reading the verbs in the order that things are > happening in the code, it's actually convert then execute. But again, > maybe some Native Speaker will have a say here, or maybe your proposed > name fits better in PostgreSQL. I'd leave it for commiter :) Agreed. I also prefer pg_read_file_all rather than pg_read_whole_file :P -- Itagaki Takahiro
pg_execute_from_file.v9.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
