Обсуждение: "Re: Question about grant create on database and pg_dump/pg_dumpall
<div dir="ltr"><span style="font-size:12.8px">On Tue, Jul 5, 2016 at 06:39 AM, Haribabu Kommi</span><br style="font-size:12.8px"/><span style="font-size:12.8px">kommi(dot)haribabu(at)gmail(</span><span style="font-size:12.8px">do<wbr/>t)com wrote:</span><br style="font-size:12.8px" /><br style="font-size:12.8px" /><blockquotestyle="font-size:12.8px;margin:0px 0px 0px 40px;border:none;padding:0px">Still i feel the GRANT statements shouldbe present, as the create<br />database statement<br />is generated only with -C option. So attached patch producesthe GRANT<br />statements based<br />on the -x option.</blockquote><br style="font-size:12.8px" /><span style="font-size:12.8px">Theattached patch does the job fine. However, I am a little skeptical about this addition, since,it is clearly mentioned in the documentation of pg_dump that it would not restore global objects, then why expectingthis. This addditon makes pg_dump -C somewhat special as now it is restoring these grant statements. Only if weconsider the popular method of dump-restore mentioned in the thread (</span><a href="https://www.postgresql.org/message-id/E1VYMqi-0001P4-P4%40wrigleys.postgresql.org"style="font-size:12.8px" target="_blank">https://www.postgresql.org/me<wbr/>ssage-id/E1VYMqi-0001P4-P4%40w<wbr />rigleys.postgresql.org</a><span style="font-size:12.8px">)with pg_dumpall -g and then individual pg_dump, then it would be helpful to have this patch.</span><brstyle="font-size:12.8px" /><div style="font-size:12.8px"><br /></div><div style="font-size:12.8px"><spanstyle="font-size:12.8px">Regards,</span></div><div style="font-size:12.8px"><span style="font-size:12.8px">RafiaSabih</span></div><div style="font-size:12.8px"><span style="font-size:12.8px">EnterpriseDB:</span><spanstyle="color:rgb(80,0,80);font-size:12.8px">: </span><a href="http://www.enterprisedb.com/"rel="noreferrer" style="font-size:12.8px" target="_blank">http://www.ente<wbr />rprisedb.com</a></div></div>
On Mon, Sep 26, 2016 at 2:29 PM, Rafia Sabih <rafia.sabih@enterprisedb.com> wrote:
On Tue, Jul 5, 2016 at 06:39 AM, Haribabu Kommi
kommi(dot)haribabu(at)gmail(dot)com wrote: Still i feel the GRANT statements should be present, as the create
database statement
is generated only with -C option. So attached patch produces the GRANT
statements based
on the -x option.
The attached patch does the job fine. However, I am a little skeptical about this addition, since, it is clearly mentioned in the documentation of pg_dump that it would not restore global objects, then why expecting this. This addditon makes pg_dump -C somewhat special as now it is restoring these grant statements. Only if we consider the popular method of dump-restore mentioned in the thread (https://www.postgresql.org/message-id/E1VYMqi-0001P4-P4%40w rigleys.postgresql.org) with pg_dumpall -g and then individual pg_dump, then it would be helpful to have this patch.
Thanks for your comments.
I am also not sure whether pg_dumpall -g and then individual pg_dump
is the more widely used approach or not? If it is the case, it is better
to fix the grant statements with -C option, otherwise I agree that
this patch is not required.
Any opinions from other members?
Hari Babu
Fujitsu Australia
On Thu, Sep 29, 2016 at 4:04 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote: > I am also not sure whether pg_dumpall -g and then individual pg_dump > is the more widely used approach or not? That's the approach I normally recommend. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > On Thu, Sep 29, 2016 at 4:04 AM, Haribabu Kommi > <kommi.haribabu@gmail.com> wrote: >> I am also not sure whether pg_dumpall -g and then individual pg_dump >> is the more widely used approach or not? > That's the approach I normally recommend. The fundamental thing we have to do in order to move forward on this is to rethink what's the division of labor between pg_dump and pg_dumpall. I find the patch as presented quite unacceptable because it's made no effort to do that (or even to touch the documentation). What do people think of this sketch: 1. pg_dump without --create continues to do what it does today, ie it just dumps objects within the database, assuming that database-level properties will already be set correctly for the target database. 2. pg_dump with --create creates the target database and also sets all database-level properties (ownership, ACLs, ALTER DATABASE SET, etc etc). 3. pg_dumpall loses all code relating to individual-database creation and property setting and instead relies on pg_dump --create to do that. This would leave only the code relating to "pg_dumpall -g" (ie, dump roles and tablespaces) within pg_dumpall itself. One thing that would still be messy is that presumably "pg_dumpall -g" would issue ALTER ROLE SET commands, but it's unclear what to do with ALTER ROLE IN DATABASE SET commands. Should those become part of "pg_dump --create"'s charter? It seems like not, but I'm not certain. Another thing that requires some thought is that pg_dumpall is currently willing to dump ACLs and other properties for template1/template0, though it does not invoke pg_dump on them. If we wanted to preserve that behavior while still moving the code that does those things to pg_dump, pg_dump would have to grow an option that would let it do that. But I'm not sure how much of that behavior is actually sensible. This would probably take a pg_dump archive version bump, since I think we don't currently record enough information for --create to do this (and we can't just cram the extra commands into the DATABASE entry, since we don't know whether --create will be specified to pg_restore). But we've done those before. Thoughts? Is there a better way to look at this? regards, tom lane
On Fri, Sep 30, 2016 at 12:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > The fundamental thing we have to do in order to move forward on this is > to rethink what's the division of labor between pg_dump and pg_dumpall. > I find the patch as presented quite unacceptable because it's made no > effort to do that (or even to touch the documentation). Patch marked as returned with feedback for now. > 1. pg_dump without --create continues to do what it does today, ie it just > dumps objects within the database, assuming that database-level properties > will already be set correctly for the target database. > > 2. pg_dump with --create creates the target database and also sets all > database-level properties (ownership, ACLs, ALTER DATABASE SET, etc etc). > > 3. pg_dumpall loses all code relating to individual-database creation > and property setting and instead relies on pg_dump --create to do that. > This would leave only the code relating to "pg_dumpall -g" (ie, dump roles > and tablespaces) within pg_dumpall itself. I saw a couple of people willing to have that on those mailling lists over the years... So +1 for the idea. > One thing that would still be messy is that presumably "pg_dumpall -g" > would issue ALTER ROLE SET commands, but it's unclear what to do with > ALTER ROLE IN DATABASE SET commands. Should those become part of > "pg_dump --create"'s charter? It seems like not, but I'm not certain. I guess that we need to think about simplifying the restore flow if we have the occasion: 1) First restore the result of pg_dumpall -g 2) Restore the state of each database's dump --create pg_dump --create assigns the created database to its rightful owner, so it assumes that the role has been already created. If we do not include those commands in pg_dump --create we make the whole restore scenario more complicated. > Another thing that requires some thought is that pg_dumpall is currently > willing to dump ACLs and other properties for template1/template0, though > it does not invoke pg_dump on them. If we wanted to preserve that > behavior while still moving the code that does those things to pg_dump, > pg_dump would have to grow an option that would let it do that. But > I'm not sure how much of that behavior is actually sensible. An extra option looks like the way to go. There are people that willingly set up ACLs and we could just document that something like pg_dump --template needs to be run to generate the template's dump values. > This would probably take a pg_dump archive version bump, since I think > we don't currently record enough information for --create to do this > (and we can't just cram the extra commands into the DATABASE entry, > since we don't know whether --create will be specified to pg_restore). > But we've done those before. Yep. (By the way, last time I saw dump/restore integration into an internal system we have finished by using pg_dumpall -g and then pg_dump on a given DB for simplicity) -- Michael
On Thu, Sep 29, 2016 at 11:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > The fundamental thing we have to do in order to move forward on this is > to rethink what's the division of labor between pg_dump and pg_dumpall. > I find the patch as presented quite unacceptable because it's made no > effort to do that (or even to touch the documentation). > > What do people think of this sketch: > > 1. pg_dump without --create continues to do what it does today, ie it just > dumps objects within the database, assuming that database-level properties > will already be set correctly for the target database. > > 2. pg_dump with --create creates the target database and also sets all > database-level properties (ownership, ACLs, ALTER DATABASE SET, etc etc). > > 3. pg_dumpall loses all code relating to individual-database creation > and property setting and instead relies on pg_dump --create to do that. > This would leave only the code relating to "pg_dumpall -g" (ie, dump roles > and tablespaces) within pg_dumpall itself. Seems like a good sketch. > One thing that would still be messy is that presumably "pg_dumpall -g" > would issue ALTER ROLE SET commands, but it's unclear what to do with > ALTER ROLE IN DATABASE SET commands. Should those become part of > "pg_dump --create"'s charter? It seems like not, but I'm not certain. I could go either way on this. > Another thing that requires some thought is that pg_dumpall is currently > willing to dump ACLs and other properties for template1/template0, though > it does not invoke pg_dump on them. If we wanted to preserve that > behavior while still moving the code that does those things to pg_dump, > pg_dump would have to grow an option that would let it do that. But > I'm not sure how much of that behavior is actually sensible. I'm not sure, either, but it's usually bad when dump-and-restore doesn't dump-and-restore things which a user might reasonably have changed. That tends to lead to bug reports and/or pg_upgrade failures. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, Sep 29, 2016 at 11:29:09AM -0400, Tom Lane wrote: > The fundamental thing we have to do in order to move forward on this is > to rethink what's the division of labor between pg_dump and pg_dumpall. > I find the patch as presented quite unacceptable because it's made no > effort to do that (or even to touch the documentation). > > What do people think of this sketch: This looks good so far. It covers most of the problems from TODO item "Refactor handling of database attributes between pg_dump and pg_dumpall". > 1. pg_dump without --create continues to do what it does today, ie it just > dumps objects within the database, assuming that database-level properties > will already be set correctly for the target database. dumpDatabase() isn't fully committed to that doctrine today; it skips most database properties, but it does dump COMMENT ON DATABASE and any pg_shseclabel entry for the database. Would you make it stop doing that? > One thing that would still be messy is that presumably "pg_dumpall -g" > would issue ALTER ROLE SET commands, but it's unclear what to do with > ALTER ROLE IN DATABASE SET commands. Should those become part of > "pg_dump --create"'s charter? It seems like not, but I'm not certain. "pg_dump --create" should emit ALTER ROLE IN DATABASE SET. Regeardless, pg_dump needs the roles in place for object ownership and ALTER DEFAULT PRIVILEGES. The latter is conceptually similar to ALTER ROLE IN DATABASE SET. > Another thing that requires some thought is that pg_dumpall is currently > willing to dump ACLs and other properties for template1/template0, though > it does not invoke pg_dump on them. If we wanted to preserve that > behavior while still moving the code that does those things to pg_dump, > pg_dump would have to grow an option that would let it do that. But > I'm not sure how much of that behavior is actually sensible. It's appropriate to restore the template1/template0 attributes one can cover without connecting to the database (pg_database, pg_db_role_setting, pg_shseclabel, pg_shdescription). template0 modifications that require a connection are essentially unsupported, so ignore those. I think, ideally, a full-cluster dump should include template1 contents. For template1, then, one would want pg_dump to emit everything except the CREATE DATABASE statement. That's a regrettable wart. Thanks, nm