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
-
Add pg_dumpall --rows-per-insert
- a193cbec1199 12.0 landed
-
pg_dump: allow multiple rows per insert
- 7e413a0f82c8 12.0 landed