Overall the patch looks good, but I did notice a few small things:
1. In pg_dumpall.c, the section /* Add long options to the pg_dump argument list */, we are now
passing along the --drop-cascade option. However, --clean is not passed in, so
any call to pg_dumpall using --drop-cascade fails a the pg_dump step. You'll note
that --if-exists it not passed along either; because we are dropping the whole database, we don't
need to have pg_dump worry about dropping objects at all. So I think that
--drop-cascade should NOT be passed along from pg_dumpall to pg_dump.
2. I'm not even sure if --drop-cascade makes sense for pg_dumpall, as you cannot cascade global things like databases
androles.
3. In the file pg_backup_archiver.c, the patch does a
stmtEnd = strstr(mark + strlen(buffer), ";");" and then spits
out things "past" the semicolon as the final %s in the appendPQExpBuffer line.
I'm not clear why: are we expecting more things to appear after the semi-colon?
Why not just append a "\n" manually as part of the previous %s?
Cheers,
Greg
The new status of this patch is: Waiting on Author
