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? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers