Re: BUG #8695: Reloading dump fails at COMMENT ON EXTENSION plpgsql

* Bruce Momjian wrote:

> On Sun, Dec 22, 2013 at 01:56:13AM +0000, chris(at)chrullrich(dot)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