Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Mon, Sep 11, 2017 at 6:41 PM, Michael Paquier wrote: > On Mon, Sep 11, 2017 at 11:43 PM, Robert Haas wrote: >> So I think this is just an excuse for turning --no-security-labels >> into --no-object-property=security-label. To me, that's just plain >> worse. > > It does not seem that my thoughts here have been correctly transmitted > to your brain. Dang, we really should have put more work into that inter-brain link. PostgreSQL group mind FTW! > I do not mean to change the user-facing options, just > to refactor the code internally so as --no-foo switches can be more > easily generated, added and handled as they are associated with an > object type. A portion of the complains is caused by the fact that a > lot of similar code is duplicated. Ah, well. No objection to refactoring away duplicate code, of course. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Mon, Sep 11, 2017 at 11:43 PM, Robert Haas wrote: > So I think this is just an excuse for turning --no-security-labels > into --no-object-property=security-label. To me, that's just plain > worse. It does not seem that my thoughts here have been correctly transmitted to your brain. I do not mean to change the user-facing options, just to refactor the code internally so as --no-foo switches can be more easily generated, added and handled as they are associated with an object type. A portion of the complains is caused by the fact that a lot of similar code is duplicated. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Sun, Sep 10, 2017 at 6:25 PM, Stephen Frost wrote: > * Michael Paquier (michael.paqu...@gmail.com) wrote: >> As there begins to be many switches of this kind and much code >> duplication, I think that some refactoring into a more generic switch >> infrastructure would be nicer. > > I have been thinking about this also and agree that it would be nicer, > but then these options would just become "shorthand" for the generic > switch. I don't really like the "generic switch infrastructure" concept. I think it will make specifying behaviors harder without any corresponding benefit. There are quite a few --no-xxx switches now but the total number of them that we could conceivably end up with doesn't seem to be a lot bigger than what we have already. Now, if we want switches to exclude a certain kind of object (e.g. table, function, text search configuration) from the backup altogether, that should be done in some generic way, like --exclude-object-type=table. But that's not what this is about. This is about excluding a certain kind of property (comments, in this case) from being backed up. And an individual kind of object doesn't have many more properties than what we already handle. So I think this is just an excuse for turning --no-security-labels into --no-object-property=security-label. To me, that's just plain worse. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
Michael, * Michael Paquier (michael.paqu...@gmail.com) wrote: > As there begins to be many switches of this kind and much code > duplication, I think that some refactoring into a more generic switch > infrastructure would be nicer. I have been thinking about this also and agree that it would be nicer, but then these options would just become "shorthand" for the generic switch. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Thu, Sep 7, 2017 at 1:43 AM, Robert Haas wrote: > On Wed, Sep 6, 2017 at 12:26 PM, Simon Riggs wrote: I'd personally be fine with --no-whatever for any whatever that might be a subsidiary property of database objects. We've got --no-security-labels, --no-tablespaces, --no-owner, and --no-privileges already, so what's wrong with --no-comments? (We've also got --no-publications; I think it's arguable whether that is the same kind of thing.) >>> >>> And --no-subscriptions in the same bucket. >> >> Yes, it is. I was suggesting that we remove those as well. FWIW, I do too. They are useful for given application code paths. > That seems like a non-starter to me. I have used those options many > times to solve real problems, and I'm sure other people have as well. > We wouldn't have ended up with all of these options if users didn't > want to control such things. As there begins to be many switches of this kind and much code duplication, I think that some refactoring into a more generic switch infrastructure would be nicer. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Wed, Sep 6, 2017 at 12:26 PM, Simon Riggs wrote: >>> I'd personally be fine with --no-whatever for any whatever that might >>> be a subsidiary property of database objects. We've got >>> --no-security-labels, --no-tablespaces, --no-owner, and >>> --no-privileges already, so what's wrong with --no-comments? >>> >>> (We've also got --no-publications; I think it's arguable whether that >>> is the same kind of thing.) >> >> And --no-subscriptions in the same bucket. > > Yes, it is. I was suggesting that we remove those as well. That seems like a non-starter to me. I have used those options many times to solve real problems, and I'm sure other people have as well. We wouldn't have ended up with all of these options if users didn't want to control such things. > But back to the main point which is that --no-comments discards ALL > comments simply to exclude one pointless and annoying comment. That > runs counter to our stance that we do not allow silent data loss. I > want to solve the problem too. I accept that not everyone uses > comments, but if they do, spilling them all on the floor is a user > visible slip up that we should not be encouraging. Sorry y'all. /me shrugs. I agree that there could be better solutions to the original problem, but I think there's no real argument that a user can't exclude comments from a backup if they wish to do so. As the OP already pointed out, it can still be done without the switch; it's just more annoying. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On 1 September 2017 at 22:08, Michael Paquier wrote: > On Sat, Sep 2, 2017 at 1:53 AM, Robert Haas wrote: >> On Mon, Aug 21, 2017 at 5:30 PM, Simon Riggs wrote: >>> Thinking ahead, are we going to add a new --no-objecttype switch every >>> time someone wants it? >> >> I'd personally be fine with --no-whatever for any whatever that might >> be a subsidiary property of database objects. We've got >> --no-security-labels, --no-tablespaces, --no-owner, and >> --no-privileges already, so what's wrong with --no-comments? >> >> (We've also got --no-publications; I think it's arguable whether that >> is the same kind of thing.) > > And --no-subscriptions in the same bucket. Yes, it is. I was suggesting that we remove those as well. But back to the main point which is that --no-comments discards ALL comments simply to exclude one pointless and annoying comment. That runs counter to our stance that we do not allow silent data loss. I want to solve the problem too. I accept that not everyone uses comments, but if they do, spilling them all on the floor is a user visible slip up that we should not be encouraging. Sorry y'all. -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Sat, Sep 2, 2017 at 1:53 AM, Robert Haas wrote: > On Mon, Aug 21, 2017 at 5:30 PM, Simon Riggs wrote: >> Thinking ahead, are we going to add a new --no-objecttype switch every >> time someone wants it? > > I'd personally be fine with --no-whatever for any whatever that might > be a subsidiary property of database objects. We've got > --no-security-labels, --no-tablespaces, --no-owner, and > --no-privileges already, so what's wrong with --no-comments? > > (We've also got --no-publications; I think it's arguable whether that > is the same kind of thing.) And --no-subscriptions in the same bucket. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Mon, Aug 21, 2017 at 5:30 PM, Simon Riggs wrote: > Thinking ahead, are we going to add a new --no-objecttype switch every > time someone wants it? I'd personally be fine with --no-whatever for any whatever that might be a subsidiary property of database objects. We've got --no-security-labels, --no-tablespaces, --no-owner, and --no-privileges already, so what's wrong with --no-comments? (We've also got --no-publications; I think it's arguable whether that is the same kind of thing.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Mon, Aug 21, 2017 at 2:30 PM, Simon Riggs wrote: > > > The patch applies cleanly to current master and all tests run without > > failures. > > > > I also test against all current supported versions (9.2 ... 9.6) and > didn't > > find any issue. > > > > Changed status to "ready for commiter". > > I get the problem, but not this solution. It's too specific and of > zero other value, yet not even exactly specific to the issue. We > definitely don't want --no-extension-comments, but --no-comments > removes ALL comments just to solve a weird problem. (Meta)Data loss, > surely? > It was argued, successfully IMO, to have this capability independent of any particular problem to be solved. In that context I haven't given the proposed implementation much thought. I do agree that this is a very unappealing solution for the stated problem and am hoping someone will either have an ingenious idea to solve it the right way or is willing to hold their nose and put in a hack. David J.
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On 7 August 2017 at 16:14, Fabrízio de Royes Mello wrote: > > On Mon, Aug 7, 2017 at 10:43 AM, Robins Tharakan wrote: >> >> On 20 July 2017 at 05:14, Robins Tharakan wrote: >>> >>> On 20 July 2017 at 05:08, Michael Paquier >>> wrote: On Wed, Jul 19, 2017 at 8:59 PM, Fabrízio de Royes Mello > You should add the properly sgml docs for this pg_dumpall change also. Tests of pg_dump go to src/bin/pg_dump/t/ and tests for objects in extensions are in src/test/modules/test_pg_dump, but you just care about the former with this patch. And if you implement some new tests, look at the other tests and base your work on that. >>> >>> >>> Thanks Michael / >>> Fabrízio. >>> >>> Updated patch (attached) additionally adds SGML changes for pg_dumpall. >>> (I'll try to work on the tests, but sending this >>> nonetheless >>> ). >>> >> >> Attached is an updated patch (v4) with basic tests for pg_dump / >> pg_dumpall. >> (Have zipped it since patch size jumped to ~40kb). >> > > The patch applies cleanly to current master and all tests run without > failures. > > I also test against all current supported versions (9.2 ... 9.6) and didn't > find any issue. > > Changed status to "ready for commiter". I get the problem, but not this solution. It's too specific and of zero other value, yet not even exactly specific to the issue. We definitely don't want --no-extension-comments, but --no-comments removes ALL comments just to solve a weird problem. (Meta)Data loss, surely? Thinking ahead, are we going to add a new --no-objecttype switch every time someone wants it? It would make more sense to add something more general and extensible such as --exclude-objects=comment or similar name -- Simon Riggshttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Mon, Aug 7, 2017 at 10:43 AM, Robins Tharakan wrote: > > On 20 July 2017 at 05:14, Robins Tharakan wrote: >> >> On 20 July 2017 at 05:08, Michael Paquier wrote: >>> >>> On Wed, Jul 19, 2017 at 8:59 PM, >>> Fabrízio de Royes Mello >>> > You should add the properly sgml docs for this pg_dumpall change also. >>> >>> Tests of pg_dump go to src/bin/pg_dump/t/ and tests for objects in >>> extensions are in src/test/modules/test_pg_dump, but you just care >>> about the former with this patch. And if you implement some new tests, >>> look at the other tests and base your work on that. >> >> >> Thanks Michael / >> Fabrízio. >> >> Updated patch (attached) additionally adds SGML changes for pg_dumpall. >> (I'll try to work on the tests, but sending this >> nonetheless >> ). >> > > Attached is an updated patch (v4) with basic tests for pg_dump / pg_dumpall. > (Have zipped it since patch size jumped to ~40kb). > The patch applies cleanly to current master and all tests run without failures. I also test against all current supported versions (9.2 ... 9.6) and didn't find any issue. Changed status to "ready for commiter". Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Timbira: http://www.timbira.com.br >> Blog: http://fabriziomello.github.io >> Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello >> Github: http://github.com/fabriziomello
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On 20 July 2017 at 05:08, Michael Paquier wrote: > On Wed, Jul 19, 2017 at 8:59 PM, > > Fabrízio de Royes Mello > > You should add the properly sgml docs for this pg_dumpall change also. > > Tests of pg_dump go to src/bin/pg_dump/t/ and tests for objects in > extensions are in src/test/modules/test_pg_dump, but you just care > about the former with this patch. And if you implement some new tests, > look at the other tests and base your work on that. > Thanks Michael / Fabrízio. Updated patch (attached) additionally adds SGML changes for pg_dumpall. (I'll try to work on the tests, but sending this nonetheless ). - robins pgdump_nocomments_v3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Wed, Jul 19, 2017 at 8:59 PM, Fabrízio de Royes Mello wrote: > On Wed, Jul 19, 2017 at 3:54 PM, Robins Tharakan wrote: >> You may want to consider this patch (attached) which additionally has the >> pg_dumpall changes. >> It would be great if you could help with the tests though, am unsure how >> to go about them. > > You should add the properly sgml docs for this pg_dumpall change also. Tests of pg_dump go to src/bin/pg_dump/t/ and tests for objects in extensions are in src/test/modules/test_pg_dump, but you just care about the former with this patch. And if you implement some new tests, look at the other tests and base your work on that. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Wed, Jul 19, 2017 at 3:54 PM, Robins Tharakan wrote: > > > On 18 July 2017 at 23:55, David Fetter wrote: >> >> Excellent point about pg_dumpall. I'll see what I can draft up in the >> next day or two and report back. > > > > Hi David, > > You may want to consider this patch (attached) which additionally has the pg_dumpall changes. > It would be great if you could help with the tests though, am unsure how to go about them. > You should add the properly sgml docs for this pg_dumpall change also. Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Timbira: http://www.timbira.com.br >> Blog: http://fabriziomello.github.io >> Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello >> Github: http://github.com/fabriziomello
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On 18 July 2017 at 23:55, David Fetter wrote: > > Excellent point about pg_dumpall. I'll see what I can draft up in the > next day or two and report back. Hi David, You may want to consider this patch (attached) which additionally has the pg_dumpall changes. It would be great if you could help with the tests though, am unsure how to go about them. - robins pgdump_nocomments_v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Tue, Jul 18, 2017 at 08:38:25AM +0200, Michael Paquier wrote: > On Tue, Jul 18, 2017 at 3:45 AM, David Fetter wrote: > > The one I run into frequently is in a proprietary fork, RDS Postgres. > > It'll happily dump out COMMENT ON EXTENSION plpgsq IS ... > > which is great as far as it goes, but errors out when you try to > > reload it. > > > > While bending over backwards to support proprietary forks strikes me > > as a terrible idea, I'd like to enable pg_dump to produce and consume > > ToCs just as pg_restore does with its -l/-L options. This would > > provide the finest possible grain. > > Let's add as well a similar switch to pg_dumpall that gets pushed down > to all the created pg_dump commands. If this patch gets integrated > as-is this is going to be asked. And tests would be welcome. Excellent point about pg_dumpall. I'll see what I can draft up in the next day or two and report back. Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Tue, Jul 18, 2017 at 3:45 AM, David Fetter wrote: > The one I run into frequently is in a proprietary fork, RDS Postgres. > It'll happily dump out COMMENT ON EXTENSION plpgsq IS ... > which is great as far as it goes, but errors out when you try to > reload it. > > While bending over backwards to support proprietary forks strikes me > as a terrible idea, I'd like to enable pg_dump to produce and consume > ToCs just as pg_restore does with its -l/-L options. This would > provide the finest possible grain. Let's add as well a similar switch to pg_dumpall that gets pushed down to all the created pg_dump commands. If this patch gets integrated as-is this is going to be asked. And tests would be welcome. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Thu, Jun 01, 2017 at 10:05:09PM -0400, Tom Lane wrote: > Robert Haas writes: > > On Tue, May 30, 2017 at 8:55 PM, David G. Johnston > > wrote: > >>> Having --no-comments seems generally useful to me, in any case. > > >> It smacks of being excessive to me. > > > It sounds perfectly sensible to me. It's not exactly an elegant > > solution to the original problem, but it's a reasonable switch on > > its own merits. > > I dunno. What's the actual use-case, other than as a bad workaround > to a problem we should fix a different way? The one I run into frequently is in a proprietary fork, RDS Postgres. It'll happily dump out COMMENT ON EXTENSION plpgsq IS ... which is great as far as it goes, but errors out when you try to reload it. While bending over backwards to support proprietary forks strikes me as a terrible idea, I'd like to enable pg_dump to produce and consume ToCs just as pg_restore does with its -l/-L options. This would provide the finest possible grain. Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: tested, passed Spec compliant: not tested Documentation:tested, passed It's a very simple change and I have not to complain about source and documentation changes. But I wonder the lack of tap tests of this new pg_dump behavior. Shouldn't we add tap tests? The new status of this patch is: Waiting on Author -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Thu, Jun 1, 2017 at 10:05 PM, Tom Lane wrote: > I dunno. What's the actual use-case, other than as a bad workaround > to a problem we should fix a different way? Well, that's a fair point. I don't have a specific use case in mind. However, I also don't think that options for controlling what gets dumped should be subjected to extreme vetting. On the strength mostly of my own experiences trying to solve database problems in the real world, I generally think that pg_dump could benefit from significantly more options to control what gets dumped. The existing options are pretty good, but it's not that hard to run into a situation that they don't cover. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Robert Haas writes: > > On Tue, May 30, 2017 at 8:55 PM, David G. Johnston > > wrote: > >>> Having --no-comments seems generally useful to me, in any case. > > >> It smacks of being excessive to me. > > > It sounds perfectly sensible to me. It's not exactly an elegant > > solution to the original problem, but it's a reasonable switch on its > > own merits. > > I dunno. What's the actual use-case, other than as a bad workaround > to a problem we should fix a different way? Perhaps it's a bit of a stretch, I'll admit, but certainly "minmization" and "obfuscation" come to mind, which are often done in other fields and might well apply in very specific cases to PG schemas. I can certainly also see a case being made that you'd like to extract a schema-only dump which doesn't include any comments because the comments have information that you'd rather not share publicly, while the schema itself is fine to share. Again, a bit of a stretch, but not unreasonable. Otherwise, well, for my 2c anyway, feels like it's simply fleshing out the options which correspond to the different components of an object. We provide similar for ACLs, security labels, and tablespace association. If there are other components of an object which we should consider adding an option to exclude, I'm all ears, personally (indexes?). Also, with the changes that I've made to pg_dump, I'm hopeful that such options will end up requiring a very minor amount of code to implement. There's more work to be done in that area too, certainly, but I do feel like it's better than it was. I definitely would like to see more flexibility in this area in general. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
Robert Haas writes: > On Tue, May 30, 2017 at 8:55 PM, David G. Johnston > wrote: >>> Having --no-comments seems generally useful to me, in any case. >> It smacks of being excessive to me. > It sounds perfectly sensible to me. It's not exactly an elegant > solution to the original problem, but it's a reasonable switch on its > own merits. I dunno. What's the actual use-case, other than as a bad workaround to a problem we should fix a different way? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Tue, May 30, 2017 at 8:55 PM, David G. Johnston wrote: >> Having --no-comments seems generally useful to me, in any case. > > It smacks of being excessive to me. It sounds perfectly sensible to me. It's not exactly an elegant solution to the original problem, but it's a reasonable switch on its own merits. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
David, * David G. Johnston (david.g.johns...@gmail.com) wrote: > On Tue, May 30, 2017 at 8:41 PM, Stephen Frost wrote: > > * David G. Johnston (david.g.johns...@gmail.com) wrote: > > > On Fri, May 26, 2017 at 7:47 AM, Stephen Frost > > wrote: > > > > * Robins Tharakan (thara...@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. Fair enough. I tend to agree with that sentiment. > 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. That would be most unfortunate, I agree. > 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. Good, then we can at least consider this particular feature as being one we're generally interested in and move forward with it, even if we also, perhaps, come up with a better solution to the specific issue at hand. > > > > 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). Perhaps, but I'm not convinced that such speculation really helps move us forward. > > > 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. I'm not entirely sure what I was getting at above, to be honest, but I believe we're generally on the same page here. I certainly agree that users don't expect a pg_dump to not be able to be successfully restored. What I may have been getting at is simply that it's not about a lack of COMMENT IF NOT EXISTS, in an ideal world, but perhaps that's what we need to solve this particular issue, for now. > 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. Right, I tend to follow your line of thought here. > > 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. Ah, but pg_dump could potentially dump out more complicated logic than it does today. We currently presume that there is never any need to reason about the results of a prior command before executing the next in pg_dump's output. In some ways, it's rather impressive that we've gotten this far with that assumption, but ensuring that is the case means that our users are also able to rely on that and write simple scripts which can be rerun to reset the database to a specific state. > During restore you have pg_restore+psql - and having cross-statement logic > in those applications is likely a non-starter. It would certainly be a very large shift from what we generate today. :) > That is ultimately the > problem here, and which is indeed fixed by the outstand
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
Stephen, On Tue, May 30, 2017 at 8:41 PM, Stephen Frost wrote: > David, > > * David G. Johnston (david.g.johns...@gmail.com) wrote: > > On Fri, May 26, 2017 at 7:47 AM, Stephen Frost > wrote: > > > * Robins Tharakan (thara...@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.
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
David, * David G. Johnston (david.g.johns...@gmail.com) wrote: > On Fri, May 26, 2017 at 7:47 AM, Stephen Frost wrote: > > * Robins Tharakan (thara...@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. A user who never uses large objects/blobs might feel the same way about "--no-blobs", or a user who never uses ACLs wondering why we have a "--no-privileges". In the end, these are also possible components that users may wish to have for their own reasons. What I, certainly, agree isn't ideal is requiring users to use such an option to generate a database-level dump file (assuming they have access to more-or-less all database objects) which can be restored as a non-superuser, that's why I was a bit hesitant about this particular solution to the overall problem. I do agree that if there is simply no use-case, ever, for wishing to strip comments from a database then it might be excessive to provide such an option, but I tend to feel that there is a reasonable, general, use-case for the option. > > 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. > 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. 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. That, however, would preclude the ability of pg_dump to produce a text file used for restore, unless we started to write into that text file DO blocks, which I doubt would go over very well. > COMMENT ON object TRY 'text' -- i.e., replace the word IS with TRY I'm not sure that I see why we should invent wholly new syntax for something which we already have in IF NOT EXISTS, or why this should really be viewed or considered any differently from the IF NOT EXISTS syntax. Perhaps you could elaborate as to how this is different from IF NOT EXISTS? > If the attempt to comment fails for any reason log a warning (maybe) but > otherwise ignore the result and continue on without invoking an error. In the IF NOT EXISTS case we log a NOTICE, which seems like it's what would be appropriate here also, again begging the question of it this is really different from IF NOT EXISTS in a meaningful way. > One suggestion I've seen is to simply "COMMENT ON EXTENSION plpgsql IS > NULL;" If that works in the scenarios people are currently dealing with > then I'd say we should advise that such an action be taken for those whom > wish to generate dumps that can be loaded by non-super-users. If the > affected users cannot make that work then maybe we should just remove the > comment from the extension. People can lookup "plpgsql" in the docs easily > enough and "PL/pgSQL procedural language" doesn't do anything more than > expand the acronym. Removing the comment as a way to deal with our deficiency in this area strikes me as akin to adding planner hints. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
On Fri, May 26, 2017 at 7:47 AM, Stephen Frost wrote: > Greetings, > > * Robins Tharakan (thara...@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. > 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. 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. COMMENT ON object TRY 'text' -- i.e., replace the word IS with TRY If the attempt to comment fails for any reason log a warning (maybe) but otherwise ignore the result and continue on without invoking an error. One suggestion I've seen is to simply "COMMENT ON EXTENSION plpgsql IS NULL;" If that works in the scenarios people are currently dealing with then I'd say we should advise that such an action be taken for those whom wish to generate dumps that can be loaded by non-super-users. If the affected users cannot make that work then maybe we should just remove the comment from the extension. People can lookup "plpgsql" in the docs easily enough and "PL/pgSQL procedural language" doesn't do anything more than expand the acronym. David J.
Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
Greetings, * Robins Tharakan (thara...@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. > A similar discussion had taken place earlier [1], however that stopped > (with a Todo entry) since it required more work at the backend. Well, that was one possible solution (COMMENT IF NOT EXISTS). That does seem like it'd be nice too, though what I think we really want here is something like: CREATE EXTENSION IF NOT EXISTS plpgsql ... COMMENT blah; That general capability has been asked for and discussed before, but it's complicated because we support comments on lots of objects and doesn't address other possible issues with this approach (comments aren't the only things that could exist on plpgsql, and I can't see us having a way to get every component included in a single command...). That leads to an interesting thought which we could consider, which would be represented in some crazy syntax such as: CREATE EXTENSION IF NOT EXISTS plpgsql ... ( COMMENT xyz; GRANT USAGE ON EXTENSION plpgsql whatever; ); > This patch provides a stop-gap solution until then. If the feedback is to > tackle this is by filtering comments for specific DB objects, an argument > name (for e.g. --no-extension-comments or something) would help and I could > submit a revised patch. That seems like it might be a bit too specific. > Alternatively, if there are no objections, I could submit this to the > Commitfest. Yes, please add it to the commitfest. Thanks! Stephen signature.asc Description: Digital signature
[HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
Hi, 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. A similar discussion had taken place earlier [1], however that stopped (with a Todo entry) since it required more work at the backend. This patch provides a stop-gap solution until then. If the feedback is to tackle this is by filtering comments for specific DB objects, an argument name (for e.g. --no-extension-comments or something) would help and I could submit a revised patch. Alternatively, if there are no objections, I could submit this to the Commitfest. References: [1] https://www.postgresql.org/message-id/1420.1397099637%40sss.pgh.pa.us - robins pgdump_nocomments_v1.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
