Re: BUG #8695: Reloading dump fails at COMMENT ON EXTENSION plpgsql - Mailing list pgsql-bugs
From | Christian Ullrich |
---|---|
Subject | Re: BUG #8695: Reloading dump fails at COMMENT ON EXTENSION plpgsql |
Date | |
Msg-id | 5339D0AB.3080707@chrullrich.net Whole thread Raw |
In response to | Re: BUG #8695: Reloading dump fails at COMMENT ON EXTENSION plpgsql (Bruce Momjian <bruce@momjian.us>) |
List | pgsql-bugs |
* Bruce Momjian wrote: > On Sun, Dec 22, 2013 at 01:56:13AM +0000, chris@chrullrich.net wrote: >> A non-superuser cannot reload any dump of a database that contains the >> plpgsql extension, because the dump unconditionally attempts to set the >> comment on that extension. This fails because plpgsql is owned by the >> superuser who installed it. >> I can think of one possible fix (aside from simply filtering that line from >> the dump): COMMENT could be a no-op if the requested comment is identical to >> the existing one. >> >> Another idea I had was to allow comments to be part of an extension, so that >> pg_dump would not dump them, but that does not work because pg_dump does not >> know if a comment has been changed from the original value. Not that anyone >> would ever do that. > This would certainly cause a restore to abort for a non-super-user if > psql used --set ON_ERROR_STOP=on. Any easy way to fix this? I am not > super-excited about the suggested fixes listed above. Not just with ON_ERROR_STOP, but also with --single-transaction. IMO, that is even worse, because restoring that way is a) a lot faster for non-enormous databases, and b) easier to clean up if some other error happens. Warning: Raw brain dump following. As for possible fixes, you can find an argument in favor of my no-op suggestion below, along with some more ideas that probably won't work, but the root cause is the "relative to template0" thing. As long as pg_dump does not adhere to that, the problem remains. It is impossible to make it adhere without expending large amounts of work both on development and at runtime, so that statement should at least be changed to mention that comments on extensions are included in the dump. Other than that, I think my suggestion of letting COMMENT succeed (fail silently, actually) if the comment would not change even if the command were executed is not that bad. If the command is -- in the specific situation -- idempotent, why not simply skip it? (The only situation where that might be a problem is if someone made pg_description not readable to everyone, thereby breaking all the \d..+ psql commands for nonprivileged users. In that case, such a user could possibly probe for existing comments, whatever good that would do them. It's at least imaginable that some application might abuse the comments for configuration metadata. Of course, COMMENT would have to read the existing comment, thereby failing early without leaking.) The simplest fix I can think of on further consideration is to give the database owner blanket COMMENT privilege on everything. Without checking the archives, I suspect that idea was considered and rejected when comments were introduced, because it looks rather obvious. Another idea: Per-user comments. If the user invoking COMMENT does not have the required privileges to set the global comment, a per-user comment would be set instead, closely followed by chaos and confusion. Yet another: COMMENT IF NOT EXISTS, without looking at the existing comment. Nicely self-contained, but the semantics if there is no comment yet look questionable: Loading a dump with a comment for an existing object that does not have one in the target database would fail again, just as it does now. That might be considered correct because we're trying to set a comment where none was before, but it does not pass the "relative to template0" test either if the comment existed in there and was removed in the dumped database. And one more: Extension ownership could implicitly follow database ownership. Not backward compatible; would give database owners additional privileges simply by upgrading, and would obsolete the pg_extension.extowner column. -- Christian
pgsql-bugs by date: