Re: pg_dump multi VALUES INSERT

Fabien COELHO <coelho@cri.ensmp.fr>

From: Fabien COELHO <coelho@cri.ensmp.fr>
To: David Rowley <david.rowley@2ndquadrant.com>
Cc: Surafel Temesgen <surafel3000@gmail.com>, Dmitry Dolgov <9erthalion6@gmail.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2019-02-02T08:26:08Z
Lists: pgsql-hackers
Hello David,

> Wondering if you have anything else here? I'm happy for the v13
> version to be marked as ready for committer.

I still have a few comments.

Patch applies cleanly, compiles, global & local make check are ok.

Typos and style in the doc:

 	"However, since, by default this option generates ..."
 	"However, since this option, by default, generates ..."

I'd suggest a more straightforward to my mind and ear: "However, since by 
default the option generates ..., ....", although beware that I'm not a 
native English speaker.

I do not understand why dump_inserts declaration has left the "flags for 
options" section.

I'd suggest not to rely on "atoi" because it does not check the argument 
syntax, so basically anything is accepted, eg "1O" is 1;

On "if (!dopt->do_nothing) $1 else $2;", I'd rather use a straight 
condition "if (dopt->do_nothing) $2 else $1;" (two instances).

There is a test, that is good! Charater "." should be backslashed in the 
regexpr. I'd consider also introducing limit cases: empty table, empty 
columns by creating corresponding tables and using -t repeatedly.

-- 
Fabien.


Commits

  1. Add pg_dumpall --rows-per-insert

  2. pg_dump: allow multiple rows per insert