Subject: Re: pg_dump additional options for performance
From: Simon Riggs <simon@2ndquadrant.com>
To: Stephen Frost <sfrost@snowman.net>
Cc: pgsql-patches@postgresql.org
In-Reply-To: <20080720030749.GA18846@tamriel.snowman.net>
References: <20080720030749.GA18846@tamriel.snowman.net>
Content-Type: text/plain
Date: Sun, 20 Jul 2008 05:47:09 +0100
Message-Id: <1216529229.19656.835.camel@ebony.2ndQuadrant>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit


On Sat, 2008-07-19 at 23:07 -0400, Stephen Frost wrote:
> Simon,
> 
>   I agree with adding these options in general, since I find myself
>   frustrated by having to vi huge dumps to change simple schema things.
>   A couple of comments on the patch though:
> 
>   - Conflicting option handling
>     I think we are doing our users a disservice by putting it on them to
> 	figure out exactly what:
> 	multiple object groups cannot be used together
> 	means to them.  You and I may understand what an "object group" is,
> 	and why there can be only one, but it's a great deal less clear than
> 	the prior message of
> 	options -s/--schema-only and -a/--data-only cannot be used together
> 	My suggestion would be to either list out the specific options which
> 	can't be used together, as was done previously, or add a bit of (I
> 	realize, boring) code and actually tell the user which of the
> 	conflicting options were used.
> 
>   - Documentation
> 	When writing the documentation I would stress that "pre-schema" and
> 	"post-schema" be defined in terms of PostgreSQL objects and why they
> 	are pre vs. post.
> 
>   - Technically, the patch needs to be updated slightly since another
> 	pg_dump-related patch was committed recently which also added
> 	options and thus causes a conflict.
> 
>   Beyond those minor points, the patch looks good to me.

Thanks for the review. I'll make the changes you suggest.

-- 
 Simon Riggs           www.2ndQuadrant.com
 PostgreSQL Training, Services and Support