Patch: Add --no-comments to skip COMMENTs with pg_dump

Greetings,

* 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.

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

On Fri, May 26, 2017 at 7:47 AM, Stephen Frost <sfrost@snowman.net> wrote:

Greetings,

* 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.

I​t 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.

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.

I​t 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

Stephen,

On Tue, May 30, 2017 at 8:41 PM, Stephen Frost <sfrost@snowman.net> wrote:

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.

I​t 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.

David,

* David G. Johnston (david.g.johnston@gmail.com) wrote:

On Tue, May 30, 2017 at 8:41 PM, Stephen Frost <sfrost@snowman.net> wrote:

* 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.

I​t 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 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.

I agree that adding COMMENT IF NOT EXISTS (or perhaps COMMENT ... TRY)
would be simpler than changing the syntax for every command to support a
COMMENT attribute being included. The issue here, however, is what I
mentioned previously- COMMENTs aren't the only attributes of an object
and if we really want to support "CREATE OBJECT + ATTRIBUTES IF NOT
EXISTS, otherwise do nothing" then that approach simply doesn't work,
without heavily modifying our syntax to allow much greater flexibility
than we've ever had before.

That's why I was suggesting that we have a mechanism for passing a set
of sub-commands to be performed on an object *if* we end up creating it,
on the basis of CREATE ... IF NOT EXISTS.

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.​

Ah, I see that distinction. I'm wondering how it might relate to other
attributes which an object might have and if we need to have similar
options for each (or perhaps we do? I've not looked, but I don't recall
anything similar offhand).

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 ;)

That's a fair point, and might allow a quick-fix (I seriously doubt
anyone would really miss the comment we have on plpgsql today), but I'm
going to continue to push back a bit on this as I'd really like a better
solution, even if it's a bit more work. If such a solution doesn't
materialize before the last CF for PG 11, then I will support a motion
to simply remove the comment.

Thanks!

Stephen

On Tue, May 30, 2017 at 8:55 PM, David G. Johnston
<david.g.johnston@gmail.com> 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

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, May 30, 2017 at 8:55 PM, David G. Johnston
<david.g.johnston@gmail.com> 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

Tom,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, May 30, 2017 at 8:55 PM, David G. Johnston
<david.g.johnston@gmail.com> 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

On Thu, Jun 1, 2017 at 10:05 PM, Tom Lane <tgl@sss.pgh.pa.us> 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

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

On Thu, Jun 01, 2017 at 10:05:09PM -0400, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, May 30, 2017 at 8:55 PM, David G. Johnston
<david.g.johnston@gmail.com> 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 <david(at)fetter(dot)org> 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

On Tue, Jul 18, 2017 at 3:45 AM, David Fetter <david@fetter.org> 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

On Tue, Jul 18, 2017 at 08:38:25AM +0200, Michael Paquier wrote:

On Tue, Jul 18, 2017 at 3:45 AM, David Fetter <david@fetter.org> 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 <david(at)fetter(dot)org> 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

On 18 July 2017 at 23:55, David Fetter <david@fetter.org> 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​

On Wed, Jul 19, 2017 at 3:54 PM, Robins Tharakan <tharakan@gmail.com> wrote:

On 18 July 2017 at 23:55, David Fetter <david@fetter.org> 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

On Wed, Jul 19, 2017 at 8:59 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:

On Wed, Jul 19, 2017 at 3:54 PM, Robins Tharakan <tharakan@gmail.com> 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

On 20 July 2017 at 05:08, Michael Paquier <michael.paquier@gmail.com> 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

On 20 July 2017 at 05:14, Robins Tharakan <tharakan@gmail.com> wrote:

On 20 July 2017 at 05:08, Michael Paquier <michael.paquier@gmail.com>
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).

-
robins

On Mon, Aug 7, 2017 at 10:43 AM, Robins Tharakan <tharakan@gmail.com> wrote:

On 20 July 2017 at 05:14, Robins Tharakan <tharakan@gmail.com> wrote:

On 20 July 2017 at 05:08, Michael Paquier <michael.paquier@gmail.com>

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