Hello David,
I thought about this and looked into it, but I decided it didn't look
like a smart thing to do. The reason is that if --inserts sets
dump_inserts to 1 then --rows-per-insert sets it to something else
that's likely fine, but if that happens in the opposite order then the
--rows-per-insert gets overwritten with 1.
You can test before doing that!
case X:
if (opt.dump_inserts == 0)
opt.dump_inserts = 1;
// otherwise option is already set
The bad news is the order that happens is defined by the order of the
command line args.
It might be possible to make it work by having --inserts set some other
variable,
ISTM that it is enough to test whether the variable is zero.
then set dump_inserts to 1 if it's set to 0 and the other variable is
set to >= 1... but that requires another variable, which is what you
want to avoid...
I still do not understand the need for another variable.
int ninserts = 0; // default is to use copy
while (getopt...)
{
switch (...) {
case "--inserts":
if (ninserts == 0) ninserts = 1;
break;
case "--rows-per-insert":
ninserts = arg_value;
checks...
break;
...
I think it's best to have a variable per argument.
I disagree, because it adds complexity where none is needed: here the new
option is an extension of a previous one, thus the previous one just
becomes a particular case, so it seems simpler to manage it as the
particular case it is rather than a special case, creating the need for
checking the consistency and so if two variables are used.
I could get rid of the got_rows_per_insert variable, but it would
require setting the default value for rows_per_insert in the main()
function rather than in InitDumpOptions(). I thought
InitDumpOptions() looked like just the place to do this, so went with
that option. To make it work without got_rows_per_insert,
rows_per_insert would have to be 0 by default and we'd know we saw a
--rows-per-insert command line arg by the fact that rows_per_insert
was non-zero. Would you rather have it that way?
Yep, esp as rows_per_insert & dump_inserts could be the same.
The feature is not tested anywhere. I still think that there should be a
test on empty/small/larger-than-rows-per-insert tables, possibly added to
existing TAP-tests.
I was hoping to get away with not having to do that... mainly because
I've no idea how.
Hmmm. That is another question! Maybe someone will help.
--
Fabien.
