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: 4912FA4E.1090000@benedekl.tvnetwork.hu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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?

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 Content-Type Size
pg_dump_role.patch text/x-patch 10.7 KB

In response to

Responses

Browse pgsql-hackers by date

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