Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump - Mailing list pgsql-hackers
| From | David G. Johnston |
|---|---|
| Subject | Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump |
| Date | |
| Msg-id | CAKFQuwaHtaJpiwAP=3G14wt0aeUvpEZ=rnXfhAxDAAPTufHrug@mail.gmail.com Whole thread Raw |
| In response to | Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump (Stephen Frost <sfrost@snowman.net>) |
| Responses |
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
(Stephen Frost <sfrost@snowman.net>)
|
| List | pgsql-hackers |
Stephen,
David,
* David G. Johnston (david.g.johnston@gmail.com) wrote:
> On Fri, May 26, 2017 at 7:47 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > * Robins Tharakan (tharakan@gmail.com) wrote:
> > > Attached is a patch adds a --no-comments argument to pg_dump to skip
> > > generation of COMMENT statements when generating a backup. This is
> > crucial
> > > for non-superusers to restore a database backup in a Single Transaction.
> > > Currently, this requires one to remove COMMENTs via scripts, which is
> > > inelegant at best.
> >
> > Having --no-comments seems generally useful to me, in any case.
>
> It smacks of being excessive to me.
>
I have a hard time having an issue with an option that exists to exclude
a particular type of object from being in the dump.
Excessive with respect to the problem at hand. A single comment in the dump is unable to be restored. Because of that we are going to require people to omit every comment in their database. The loss of all that information is in excess of what is required to solve the stated problem which is how I was thinking of excessive.
I agree that the general idea of allowing users to choose to include or exclude comments is worth discussion along the same lines of large objects and privileges.
> > CREATE EXTENSION IF NOT EXISTS plpgsql ... COMMENT blah;
>
> A less verbose way to add comments to objects would be nice but we have an
> immediate problem that we either need to solve or document a best practice
> for.
The above would be a solution to the immediate problem in as much as
adding COMMENT IF NOT EXISTS would be.
I believe it would take a lot longer, possibly even until 12, to get the linked comment feature committed compared to committing COMMENT IF NOT EXISTS or some variation (or putting in a hack for that matter).
> COMMENT IF NOT EXISTS has been brought up but it doesn't actually map to
> what seems to me is the underlying problem...that people don't want a
> non-functional (usually...) aspect preventing successful restoration.
I tend to disagree with this characterization- I'm of the opinion that
people are, rightly, confused as to why we bother to try and add a
COMMENT to an object which we didn't actually end up creating (as it
already existed), and then throw an error on it to boot.
My characterization does actually describe the current system though. While I won't doubt that people do hold your belief it is an underlying mis-understanding as to how PostgreSQL works since comments aren't, as you say below, actual attributes but rather objects in their own right. I would love to have someone solve the systemic problem here. But the actual complaint can probably be addressed without it.
Were pg_dump a
bit more intelligent of an application, it would realize that once the
CREATE ... IF NOT EXISTS returned a notice saying "this thing already
existed" that it would realize that it shouldn't try to adjust the
attributes of that object, as it was already existing.
pg_dump isn't in play during the restore which is where the error occurs.
During restore you have pg_restore+psql - and having cross-statement logic in those applications is likely a non-starter. That is ultimately the problem here, and which is indeed fixed by the outstanding proposal of embedding COMMENT within the CREATE/ALTER object commands. But today, comments are independent objects and solving the problem within that domain will probably prove easier than changing how the system treats comments.
> COMMENT ON object TRY 'text' -- i.e., replace the word IS with TRY
Perhaps you could elaborate as to how this is different from IF NOT
EXISTS?
If the comment on plpgsql were removed for some reason the COMMENT ON IF NOT EXISTS would fire and then it would fail due to permissions. With "TRY" whether the comment (or object for that matter) exists or not the new comment would be attempted and if the permission failure kicked in it wouldn't care.
If the
> affected users cannot make that work then maybe we should just remove the
> comment from the extension.
Removing the comment as a way to deal with our deficiency in this area
strikes me as akin to adding planner hints.
Maybe, but the proposal you are supporting has been around for years, with people generally in favor of it, and hasn't happened yet. At some point I'd rather hold my nose and fix the problem myself than wait for the professional to arrive and do it right. Any, hey, we've had multiple planner hints since 8.4 ;)
David J.
pgsql-hackers by date: