Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Fabrízio Mello <fabriziomello(at)gmail(dot)com>, Robins Tharakan <tharakan(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, David Fetter <david(at)fetter(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Subject: Re: [HACKERS] Patch: Add --no-comments to skip COMMENTs with pg_dump
Date: 2018-01-23 01:40:59
Message-ID: 20180123014059.GC2416@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert, all,

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Mon, Aug 7, 2017 at 11:14 AM, Fabrízio de Royes Mello
> <fabriziomello(at)gmail(dot)com> wrote:
> > I also test against all current supported versions (9.2 ... 9.6) and didn't
> > find any issue.
> >
> > Changed status to "ready for commiter".
>
> On a very fast read this patch looks OK to me, but I'm a bit concerned
> about whether we have consensus for it. By my count the vote is 6-3
> in favor of proceeding.
>
> +1: Robins Tharakan, Stephen Frost, David Fetter, Fabrizio Mello,
> Michael Paquier, Robert Haas
> -1: David G. Johnston, Tom Lane, Simon Riggs
>
> I guess that's probably sufficient to go forward, but does anyone wish
> to dispute my characterization of their vote or the vote tally in
> general? Would anyone else like to cast a vote?

This patch continues to drag on, so I'll try to spur it a bit.

David G. Jonston, you are listed as a '-1' on this but your last comment
on this thread was:

CAKFQuwZuYhh+KUzp76cdB4r8Z616ZwEWcHEEDq6vEqgdGP9QZw(at)mail(dot)gmail(dot)com

Where you commented:

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

Which didn't strike me as really being against this patch but rather a
complaint that making use of this feature to "fix" the non-superuser
Single Transaction restore was a hack, which is somewhat indepedent.
We have lots of features that people use in various very hacky ways, but
that doesn't mean those are bad features for us to have.

Simon, you are also a '-1' on this but your last comment also makes me
think you're unhappy with this feature being used in a hacky way but
perhaps aren't against the feature itself:

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

I really can't see accepting this as a data loss issue when this feature
is used only when requested by a user (it would *not* be enabled by
default; I'd be against that too). We have other options would also be
considered data loss with this viewpoint but I can't imagine we would be
willing to remove them (eg: --no-privileges, or perhaps more clearly
--no-blobs, which was only recently added).

Tom, you're also listed as a '-1' on this but your last comment on this
thread was 16858(dot)1496369109(at)sss(dot)pgh(dot)pa(dot)us where you asked:

> I dunno. What's the actual use-case, other than as a bad workaround
> to a problem we should fix a different way?

which was answered by both Robert and I; Robert's reply being mainly
that it's likely to prove useful in the field even if we don't have a
specific use-case today, while I outlined at least feasible use-cases
(minimization, obfuscation).

For my 2c, I don't know that a large use-case is needed for this
particular feature to begin with.

In the end, I feel like this patch has actually been through the ringer
and back because it was brought up in the context of solving a problem
that we'd all like to fix in a better way. Had it been simply dropped
on us as a "I'd like to not have comments in my pg_dump output and
here's a patch to allow me to do that" I suspect it would have been
committed without anywhere near this level of effort.

I'll just reiterate Robert's ask for people to clarify their positions.

If we're not moving forward, then the next step is to mark the patch as
Rejected.

Thanks!

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-01-23 01:50:35 Re: [HACKERS] Adding column_constraint description in ALTER TABLE synopsis
Previous Message Peter Geoghegan 2018-01-23 01:22:50 Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)