Skip site navigation (1) Skip section navigation (2)

Re: pg_dump roles support [Review]

From: Benedek László <laci(at)benedekl(dot)tvnetwork(dot)hu>
To: ibrar(dot)ahmad(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_dump roles support [Review]
Date: 2008-11-06 14:08:14
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-hackers

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

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?

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(-)

Attachment: pg_dump_role.patch
Description: text/x-patch (10.7 KB)

In response to


pgsql-hackers by date

Next:From: KaiGai KoheiDate: 2008-11-06 14:23:01
Subject: Re: The suppress_redundant_updates_trigger() works incorrectly
Previous:From: Tom LaneDate: 2008-11-06 14:01:57
Subject: Re: [PATCH] EnableDisableTrigger Cleanup & Questions

Privacy Policy | About PostgreSQL
Copyright © 1996-2018 The PostgreSQL Global Development Group