Thread: HELP! BUG? pg_dump mucks up grant/revoke
Hi:
I'm using pg 7.1.2.
I've got a database with views which have permissions granted to a certain
user. Defined something like:
create view whatever....
revoke all on whatever from user
grant select on whatever to user
(If I get the syntax wrong, it doesn't matter here.)
THEN, I do a pg_dump:
pg_dump --attribute-inserts database > dump.sql
all well and good expect for one thing: the grant/revoke lines appear
BEFORE the create view definitions, so when I read the dump back into
postgres via:
dropdb database
createdb database
cat dump.sql | psql database
I get "relation doesn't exist" errors and my "user" no longer has
permission to use those views. This is a bug, isn't it?
Keith
Keith F Irwin <kirwin14@home.com> writes:
> [ 7.1.2 pg_dump dumps GRANT/REVOKE for a view before the view itself ]
>
> This is a bug, isn't it?
Yes, and a rather unpleasant one, since it will force people to do
manual correction of their dump files, if they have views with
permissions set on them.
I think the fix will be simple (haven't looked at the code yet though).
Question we need to think about: is this a sufficiently bad problem
to justify putting out a 7.1.3 release? There are a few other
unhappinesses in 7.1.2 that have been or could be fixed by
back-patching, but so far I haven't felt that we needed a 7.1.3,
given that 7.2 isn't too far away. But this bug is going to make it
difficult for people to upgrade to 7.2, unless we provide a 7.1
patch release first.
Opinions anyone?
regards, tom lane
Keith F Irwin <kirwin14@home.com> writes:
>> [ 7.1.2 pg_dump dumps GRANT/REVOKE for a view before the view itself ]
>>
>> This is a bug, isn't it?
Attached is the patch against 7.1.2 to fix this problem.
regards, tom lane
*** src/bin/pg_dump/pg_dump.c.orig Sat May 12 19:36:44 2001
--- src/bin/pg_dump/pg_dump.c Sun Jul 29 17:31:43 2001
***************
*** 3828,3833 ****
--- 3828,3834 ----
*tok,
*eqpos,
*priv;
+ char *objoid;
char *sql;
char tmp[1024];
int sSize = 4096;
***************
*** 3908,3914 ****
free(aclbuf);
! ArchiveEntry(fout, tbinfo.oid, tbinfo.relname, "ACL", NULL, sql, "", "", "", NULL, NULL);
}
--- 3909,3920 ----
free(aclbuf);
! if (tbinfo.viewdef != NULL)
! objoid = tbinfo.viewoid;
! else
! objoid = tbinfo.oid;
!
! ArchiveEntry(fout, objoid, tbinfo.relname, "ACL", NULL, sql, "", "", "", NULL, NULL);
}
Tom Lane <tgl@sss.pgh.pa.us> wrote:
> it will force people to do manual correction of their dump files, if they
> have views with permissions set on them.
> Question we need to think about: is this a sufficiently bad problem to
> justify putting out a 7.1.3 release?
Well, if there isn't a 7.1.3 release that fixes this, there'll be a lot of
users running into this problem when they upgrade to 7.2. If there is a
7.1.3 release that fixes this, many people will not encounter this problem
as they will have upgraded to 7.1.3 (without needing a dump/restore) by the
time 7.2 is released.
I'd certainly appreciate a 7.1.3 maintenance release.
Ray
--
"A.O.L.. C.I.A.. NSA. Whatever. They all have three letters. They all
collect information. And they all screw the public."
Evil Crud Puppy in
http://www.userfriendly.org/cartoons/archives/00feb/20000210.html
Tom Lane wrote: > > Keith F Irwin <kirwin14@home.com> writes: > > [ 7.1.2 pg_dump dumps GRANT/REVOKE for a view before the view itself ] > > > > This is a bug, isn't it? > > Yes, and a rather unpleasant one, since it will force people to do > manual correction of their dump files, if they have views with > permissions set on them. > > I think the fix will be simple (haven't looked at the code yet though). > > Question we need to think about: is this a sufficiently bad problem > to justify putting out a 7.1.3 release? There are a few other > unhappinesses in 7.1.2 that have been or could be fixed by > back-patching, but so far I haven't felt that we needed a 7.1.3, > given that 7.2 isn't too far away. But this bug is going to make it > difficult for people to upgrade to 7.2, unless we provide a 7.1 > patch release first. This *is* a problem that will make it difficult for ppl to upgrade to 7.2 because everyone will have to dump/restore, unless there is a workaround made for 7.2 that would accept dumps with this bug. Not everyone would be comfortable with patching their source. A lot of people didn't build from source to begin with. Most users probably aren't subscribed to general and won't know of this patch's existance but would notice a new version being released. In any case in the release notes of 7.2 this problem should be noted. -- Joseph Shraibman jks@selectacast.net Increase signal to noise ratio. http://www.targabot.com
Tom Lane <tgl@sss.pgh.pa.us> writes: > Keith F Irwin <kirwin14@home.com> writes: > > [ 7.1.2 pg_dump dumps GRANT/REVOKE for a view before the view itself ] > > > > This is a bug, isn't it? > > Yes, and a rather unpleasant one, since it will force people to do > manual correction of their dump files, if they have views with > permissions set on them. > > I think the fix will be simple (haven't looked at the code yet though). > > Question we need to think about: is this a sufficiently bad problem > to justify putting out a 7.1.3 release? There are a few other > unhappinesses in 7.1.2 that have been or could be fixed by > back-patching, but so far I haven't felt that we needed a 7.1.3, Buggy 7.1.2 -> 7.2 will create more problems than if people get 7.1.3 and do a 7.1.3 -> 7.2 update. > given that 7.2 isn't too far away. But this bug is going to make it > difficult for people to upgrade to 7.2, unless we provide a 7.1 > patch release first. > > Opinions anyone? I'd hope for "yes", although the available patch should solve the problems for us (Red Hat Linux) as I'll add that. -- Trond Eivind Glomsrød Red Hat, Inc.
teg@redhat.com (Trond Eivind =?iso-8859-1?q?Glomsr=F8d?=) writes:
> I'd hope for "yes", although the available patch should solve the
> problems for us (Red Hat Linux) as I'll add that.
Okay. If you're doing a rerelease of pg_dump, please also pick up the
REL7_1_STABLE commits I just made a few minutes ago; those clean up a
couple other bits of breakage...
regards, tom lane
On Fri, 3 Aug 2001, Tom Lane wrote: > teg@redhat.com (Trond Eivind =?iso-8859-1?q?Glomsr=F8d?=) writes: > > I'd hope for "yes", although the available patch should solve the > > problems for us (Red Hat Linux) as I'll add that. > > Okay. If you're doing a rerelease of pg_dump, please also pick up the > REL7_1_STABLE commits I just made a few minutes ago; those clean up a > couple other bits of breakage... I'll take a look at those - we've never had a release of postgresql 7.1 before (in the distro), so it's not a "re-release". It's just making sure that users of the next release of Red Hat Linux will be able to upgrade in the future. I'm pretty sure that postgresql 7.2 won't be ready. -- Trond Eivind Glomsrød Red Hat, Inc.
On Friday 03 August 2001 16:35, Tom Lane wrote: > teg@redhat.com (Trond Eivind =?iso-8859-1?q?Glomsr=F8d?=) writes: > > I'd hope for "yes", although the available patch should solve the > > problems for us (Red Hat Linux) as I'll add that. > Okay. If you're doing a rerelease of pg_dump, please also pick up the > REL7_1_STABLE commits I just made a few minutes ago; those clean up a > couple other bits of breakage... Whoa.... A pg_dump that can't be restored should be a release forcer, IMHO. After all, we're talking about our only upgrade path here -- the last 7.1.x release _MUST_ be able to make a dump that 7.2 can reliably read! As PG 7.1 has never been released with an official Red Hat (it's in the Roswell beta, IIRC), 're-release' is a misnomer from RH's POV. I'm inclined to rerelease RPM's, though. Although I shouldn't call them '7.1.2' RPM's at that point -- although I have an intarray asynchrony now... -- Lamar Owen WGCR Internet Radio 1 Peter 4:11
Lamar Owen <lamar.owen@wgcr.org> writes:
> A pg_dump that can't be restored should be a release forcer, IMHO.
Let's not panic here. The actual net effect of these bugs will be that
the pg_dump output script fails to restore the permissions and/or
comment associated with a view. Which'd be annoying, but hardly
something to ruin your whole day.
I happen to agree that it'd be appropriate to put out a 7.1.3 now,
because of the accumulated bug fixes since 7.1.2 --- not only these
pg_dump issues, but the WAL truncation fix seems to be something that
lots of people need. I haven't been able to stir up any interest from
the rest of core, however.
regards, tom lane
> Lamar Owen <lamar.owen@wgcr.org> writes: > > A pg_dump that can't be restored should be a release forcer, IMHO. > > Let's not panic here. The actual net effect of these bugs will be that > the pg_dump output script fails to restore the permissions and/or > comment associated with a view. Which'd be annoying, but hardly > something to ruin your whole day. > > I happen to agree that it'd be appropriate to put out a 7.1.3 now, > because of the accumulated bug fixes since 7.1.2 --- not only these > pg_dump issues, but the WAL truncation fix seems to be something that > lots of people need. I haven't been able to stir up any interest from > the rest of core, however. I am interested. :-) -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Tom Lane wrote: > > Lamar Owen <lamar.owen@wgcr.org> writes: > > A pg_dump that can't be restored should be a release forcer, IMHO. > > Let's not panic here. The actual net effect of these bugs will be that > the pg_dump output script fails to restore the permissions and/or > comment associated with a view. Which'd be annoying, but hardly > something to ruin your whole day. Maybe it won't ruin your whole day, but if you are unaware of the problem, it can sure ruin a few hours. I found out about the problem maybe a day before it hit the newsgroup, but had not identified the cause -- only the effect. It turns out our live servers refer to a view, and I was stuck diagnosing the problem with a two year-old update script that was suddenly broken, while trying to fight off the people who wanted new content uploded. Combine that with a major upgrade that requires an initdb, and I'm sure you will have complaints. IMHO, it speaks well of Redhat and Linux's user-orientation that Lamar and Trond are seem to be planning to release a patched RPM. Sort of reaffirms why Linux has had some success making inroads against windows. And I begin to see why some other unix variants have been accused of hostility to their users. It won't matter to me one bit. I have already applied the patch. But If you want to protect PostgreSQL's growing reputation, a release would seem to be worthwhile, again IMHO. -- Karl
Tom Lane <tgl@sss.pgh.pa.us> writes: > Lamar Owen <lamar.owen@wgcr.org> writes: > > A pg_dump that can't be restored should be a release forcer, IMHO. > > Let's not panic here. The actual net effect of these bugs will be that > the pg_dump output script fails to restore the permissions and/or > comment associated with a view. Which'd be annoying, but hardly > something to ruin your whole day. The impact will vary from location to location... but given that this is the only way to upgrade the database (as you know, this is one of my pet peeves), it needs to work. > pg_dump issues, but the WAL truncation fix seems to be something that > lots of people need. Are those also in the 7.1-stable branch? -- Trond Eivind Glomsrød Red Hat, Inc.
teg@redhat.com (Trond Eivind =?iso-8859-1?q?Glomsr=F8d?=) writes:
> Are those also in the 7.1-stable branch?
Yes. There are quite a few fixes in the stable branch since 7.1.2,
actually. I made a list last week of commits that either were already
in REL7_1_STABLE, or looked like they deserved to be back-patched if we
were about to make a patch release. Updated, it looks like:
2001-08-03 16:14 tgl
* src/bin/pg_dump/: pg_dump.c, pg_dump.h (REL7_1_STABLE):
Back-patch fixes for dumping user-defined types and dumping
comments on views.
2001-07-31 14:39 tgl
* src/: backend/optimizer/path/allpaths.c,
backend/optimizer/util/clauses.c, backend/utils/adt/ruleutils.c,
include/optimizer/clauses.h (REL7_1_STABLE): Fix optimizer to
not try to push WHERE
clauses down into a sub-SELECT that has a DISTINCT ON clause, per
bug report from Anthony Wood. While at it, improve the
DISTINCT-ON-clause recognizer routine to not be fooled by out-
of-order DISTINCT lists. Also, back-patch earlier fix to not push
down into sub-SELECT with LIMIT.
2001-07-29 18:12 tgl
* src/bin/pg_dump/: pg_dump.c (REL7_1_STABLE), pg_dump.c: Arrange
for GRANT/REVOKE on a view to be dumped at the right time, namely
after the view definition rather than before it. Bug introduced in
7.1 by changes to dump stuff in OID ordering.
2001-07-16 13:57 tgl
* src/backend/optimizer/path/allpaths.c: Do not push down quals
into subqueries that have LIMIT/OFFSET clauses, since the added
qual could change the set of rows that get past the LIMIT. Per
discussion on pgsql-sql 7/15/01.
2001-07-11 17:53 momjian
* src/backend/commands/copy.c: Disable COPY TO/FROM on views.
2001-07-05 22:13 ishii
* doc/src/sgml/backup.sgml (REL7_1_STABLE): Fix typo. createdb -t
--> createdb -T
2001-07-03 12:49 tgl
* src/backend/utils/init/miscinit.c: Don't go into infinite loop if
/home/postgres/testversion/data directory is not writable.
2001-07-02 15:31 tgl
* src/test/regress/expected/: abstime-solaris-1947.out,
abstime.out: Update abstime expected results to match
post-30-June-2001 reality. Probably the right fix is to remove
'current' special value entirely, but I don't want to see
regression test failures until that happens.
2001-06-29 12:34 tgl
* src/backend/commands/: vacuum.c (REL7_1_STABLE), vacuum.c: Fix
longstanding error in VACUUM: sometimes would examine a buffer page
after writing/unpinning it. An actual failure is unlikely, unless
the system is tremendously short of buffers ... but a bug is a bug.
2001-06-12 21:02 tgl
* src/pl/plpgsql/src/pl_exec.c (REL7_1_STABLE): Back-patch fix for
attempt to pfree a value that's not palloc'd (it's a field of a
tuple). I see Jan has already fixed this in current sources, but
7.1.* is pretty badly broken here.
2001-06-12 14:54 tgl
* src/backend/rewrite/: rewriteHandler.c (REL7_1_STABLE),
rewriteHandler.c: Repair problem with multi-action rules in
combination with any nontrivial manipulation of rtable/jointree by
planner. Rewriter was generating actions that shared
rtable/jointree substructure, which caused havoc when planner got
to the later actions that it'd already mucked up.
2001-06-06 14:54 wieck
* src/pl/plpgsql/src/gram.y: Patch from Ian Lance Taylor fixing
multiple cursor arguments and buffer zero termination.
Jan
2001-06-06 13:18 tgl
* src/backend/access/transam/xlog.c (REL7_1_STABLE): Back-patch
change to not keep WAL segments just for UNDO information.
2001-05-31 17:49 momjian
* doc/src/sgml/: release.sgml (REL7_1_STABLE), release.sgml: Forgot
SGML section section id tag for 7.1.
2001-05-31 13:32 tgl
* src/backend/utils/adt/: ri_triggers.c (REL7_1_STABLE),
ri_triggers.c: RI triggers would fail for datatypes using old-style
equal function, because cached fmgr info contained reference to a
shorter-lived data structure. Also guard against possibility that
fmgr_info could fail, leaving an incomplete entry present in the
hash table.
2001-05-27 21:00 ishii
* src/backend/utils/mb/: conv.c (REL7_1_STABLE), conv.c: Fix a
message error in utf_to_local
regards, tom lane
Lamar Owen <lamar.owen@wgcr.org> writes: > On Friday 03 August 2001 16:35, Tom Lane wrote: > > teg@redhat.com (Trond Eivind =?iso-8859-1?q?Glomsr=F8d?=) writes: > > > I'd hope for "yes", although the available patch should solve the > > > problems for us (Red Hat Linux) as I'll add that. > > > Okay. If you're doing a rerelease of pg_dump, please also pick up the > > REL7_1_STABLE commits I just made a few minutes ago; those clean up a > > couple other bits of breakage... > > Whoa.... > > A pg_dump that can't be restored should be a release forcer, IMHO. After > all, we're talking about our only upgrade path here -- the last 7.1.x release > _MUST_ be able to make a dump that 7.2 can reliably read! > > As PG 7.1 has never been released with an official Red Hat (it's in the > Roswell beta, IIRC), 're-release' is a misnomer from RH's POV. > > I'm inclined to rerelease RPM's, though. Although I shouldn't call them > '7.1.2' RPM's at that point -- although I have an intarray asynchrony now... A set of RPMs with fixes from REL7_1_STABLE as of yesterday (and some other minor fixes, related to the rpm) should be available from http://people.redhat.com/teg/pg/ -- Trond Eivind Glomsrød Red Hat, Inc.