Hello Masahiko-san,
Yeah, once custom initialization patch get committed we can extend it.
Attached updated patch. I've incorporated the all comments from Fabien
to it, and changed it to single letter version.
Patch applies and works.
A few comments and questions about the code and documentation:
Why not allow -I as a short option for --custom-initialize?
I do not think that the --custom-initialize option needs to appear as a
specific synopsis in the documentation, as it is now a sub-option of -i.
checkCustomCmds: I would suggest to simplify test code with strchr
and to merge the two fprintf into one, something like:
if (strchr("tdpfv", *cmd) == NULL) {
fprintf(stderr,
"....\n"
"....\n",
...);
...
Moreover there is already an error message later if checkCustomCmds fails, I
think
it could be expanded and the two-line one in the function dropped
entirely? It seems strange to have two level error messages for that.
Help message looks strange. I suggest something regexp-like:
--custom-initialize=[tdvpf]+
I would suggest to put the various init* functions in a more logical order:
first create table, then load data, etc.
In case 0: do not exchange unlogged_tables & foreign_keys gratuiously.
After checking the initial code, I understand that the previous default
was "tdvp", but you put "tdpv". I'm unsure whether vaccuum would do
something to the indexes and that would make more sense. In doubt, I
suggest to keep the previous default.
Maybe --foreign-keys should really do "tdvpf"?
I may be okay with disallowing --foreign-keys and --no-vacuum if --custom-init
is used,
but then there is no need to test it again in init... I think that in any case
'f' and 'v'
should always trigger the corresponding initializations.
On the other hand, I think that it could be more pragmatic with these
options, i.e. --foreign-keys could just append 'f' to the current command
if not already there, and '--no-vacuum' could remove 'v' if there, on the
fly, so that nothing would be banned. This would require to always have a
malloced custom_init string. It would allow to remove the "foreign_keys"
global variable. "is_no_vacuum" is probably still needed for benchmarking.
This way there would be no constraints and "is_custom_init" could be
dropped as well.
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
