Hello Surafel,

okay .thank you for explaining. i attach a patch corrected as such

About this v9: applies cleanly, compiles, global and local "make check" ok.

The option is not exercise in the TAP tests. I'd suggest that it should be tested on a small table with zero, 1, more than the value set number of rows. Maybe use -t and other options to reduce the output to the minimum.

About the documentation:

 +   When using <option>--rows-per-insert</option>, this allows the maximum 
number
 +   of rows per <command>INSERT</command> statement to be specified.

I'd suggest a more direct and simple style, something like:

  Set the maximum number of rows per INSERT statement.
  This option implies --inserts.
  Default to 1.

About the help message, the new option expects an argument, but it does not show:

 +  printf(_("  --rows-per-insert    number of row per INSERT command\n"));

About the code, maybe avoid using an int as a bool, eg:

    ... && !dopt.dump_inserts_multiple)
 -> ... && dopt.dump_inserts_multiple == 0)

Spacing around operators, eg: "!=1)" -> "!= 1)"

ISTM that the "dump_inserts_multiple" field is useless, you can reuse "dump_inserts" instead, i.e. --inserts sets it to 1 *if zero*, and --rows-per-inserts=XXX sets it to XXX. That would simplify the code significantly.

ISTM that there are indentation issues, eg on added

  if (dopt->dump_inserts_multiple == 1) {

The old code is not indented properly.

--
Fabien.

Reply via email to