The general idea of this patch is not disputed, I think.
Some concerns about the details:
- Why is quote_all_identifiers left behind as a global variable?
- Shouldn't pg_dumpall also use DumpOptions?
- What about binary_upgrade?
- I think some of the variables in pg_dump's main() don't need to be
moved into DumpOptions. For example, compressLevel is only looked at
once in main() and then forgotten. We don't need to pass that around
everywhere. The same goes for dumpencoding and possibly others.
- The forward declaration of struct _dumpOptions in pg_backup.h seems
kind of useless. You could move some things around so that that's not
necessary.
- NewDumpOptions() and NewRestoreOptions() are both in
pg_backup_archiver.c, but NewRestoreOptions() is in pg_backup.h whereas
NewDumpOptions() is being put into pg_backup_archiver.h. None of that
makes too much sense, but it could be made more consistent.
- In dumpOptionsFromRestoreOptions() you are building the return value
in a local variable that is not zeroed. You should probably use
NewDumpOptions() there.
Also, looking at dumpOptionsFromRestoreOptions() and related code makes
me think that these should perhaps really be the same structure. Have
you investigated that?
