Reviewing pg_dump-rows-per-insert-option-v14.
Mostly going back over things that Fabien mentioned:
On Sat, 2 Feb 2019 at 21:26, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> 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.
+ (?:INSERT\ INTO\ dump_test.test_table\ VALUES\ \(\d,\ NULL,\ NULL,\
NULL\),\ \(\d,\ NULL,\ NULL,\ NULL\),\ \(\d,\ NULL,\ NULL,\
NULL\);\n){3}
the . here before the table name needs to be escaped. The ones missing
in the existing tests should have been fixed by d07fb6810e.
There's also the additional tests that Fabien mentions.
Also, maybe one for Fabien (because he seems keen on keeping the
--rows-per-insert validation code)
strtol() returns a long. dump_inserts is an int, so on machines where
sizeof(long) == 8 and sizeof(int) == 4 (most machines, these days) the
validation is not bulletproof. This could lead to:
$ pg_dump --rows-per-insert=2147483648
pg_dump: rows-per-insert must be a positive number
For me, I was fine with the atoi() code that the other options use,
but maybe Fabien has a problem with the long vs int?
It would be simple to workaround by assigning the strtol() to a long
and making the ERANGE test check for ERANGE or ... > PG_INT_MAX
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services