Re: pg_dump multi VALUES INSERT

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Surafel Temesgen <surafel3000(at)gmail(dot)com>
Cc: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_dump multi VALUES INSERT
Date: 2019-01-22 15:36:04
Message-ID: alpine.DEB.2.21.1901221616120.16643@lancre
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers


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.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message James Coleman 2019-01-22 15:43:02 Re: Proving IS NOT NULL inference for ScalarArrayOpExpr's
Previous Message John Naylor 2019-01-22 15:35:15 Re: Delay locking partitions during INSERT and UPDATE