On Thu, Nov 6, 2008 at 8:08 PM, Benedek László <[EMAIL PROTECTED]> wrote: > Hi, > > Thanks for your review. > > I created an updated patch according to your notices. > >> 1 - Patch does not apply cleanly on latest git repository, although >> there is no hunk failed but there are some hunk succeeded messages. > > Rebased to the current HEAD. > >> 2- Patch contains unnecessary spaces and tabs which makes the patch >> unnecessarily big. IMHO please read the patch before sending and make >> sure that patch only contains the changes you intended to send. > > Yes, there were trailing whitespaces in the original files which > were removed by the previous patch. The attached version leaves them as is. > >> 3 - We should follow the coding standards of existing code > > I tried, of course, but this escaped my observation. > >> 4 - pg_restore gives error wile restoring custom format backup >> 5 - Do you really want to write this code like this > > Fixed.
> I also need some feedback about the role support in pg_restore (not > implemented yet). > Currently pg_restore sets the role during the restore process according to > the TOC > entry in the archive. It may also support the --role option (just like > pg_dump). > If specified it can be used to cancel the effect of the TOC entry and force > the > emitting of the SET ROLE ... command. With emtpy argument it can be used to > omit > the SET ROLE even if it is specified in the archieve. What do you think? > Now this patch looks OK to me. As for as pg_restore is concern I think we should not add this option into pg_restore. What advantages do you want to get by using SET ROLE in pg_restore? > Thank you again. > > doc/src/sgml/ref/pg_dump.sgml | 16 ++++++++++ > doc/src/sgml/ref/pg_dumpall.sgml | 15 +++++++++ > src/bin/pg_dump/pg_backup.h | 2 + > src/bin/pg_dump/pg_backup_archiver.c | 36 +++++++++++++++++++++- > src/bin/pg_dump/pg_dump.c | 53 > ++++++++++++++++++++++++++++++++++ > src/bin/pg_dump/pg_dumpall.c | 23 ++++++++++++++ > 6 files changed, 143 insertions(+), 2 deletions(-) > > > -- Ibrar Ahmed EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
