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.