Re: pg_dump multi VALUES INSERT

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.