Re: bad examples in pg_dump README

Lists: pgsql-hackers
From: Josh Kupershmidt <schmiddy(at)gmail(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: bad examples in pg_dump README
Date: 2013-01-04 02:36:37
Message-ID: CAK3UJRFbnCD+EsZ1Bc8M2dq2FJ0jmkjGEAPTNPC8Xfk9FWcSrg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

The ./src/bin/pg_dump README contains several inoperable examples.
First, this suggestion:

| or to list tables:
|
| pg_restore <backup-file> --table | less

seems bogus, i.e. the --table option requires an argument specifing
which table to restore, and does not "list tables". Next, this
suggested command:

| pg_restore <backup-file> -l --oid --rearrange | less

hasn't worked since 7.4 or thereabouts, since we don't have
--rearrange or --oid anymore. (I'm not sure we ever had --oid, maybe
it was supposed to be --oid-order?). Then, three examples claim we
support a "--use" flag, e.g.

| pg_restore backup.bck --use=toc.lis > script.sql

where presumably "--use-list" was meant. Further little gripes with
this README include mixed use of tabs and spaces for the examples, and
lines running over the 80-column limit which at least some of our
other READMEs seem to honor.

I started out attempting to fix up the README, keeping the original
example commands intact, but upon further reflection I think the
examples of dumping, restoring, and selective-restoring in that REAMDE
don't belong there anyway. There are already better examples of such
usage in the pg_dump/pg_restore documentation pages, and IMO that's
where such generally-useful usage information belongs.

I propose slimming down the pg_dump README, keeping intact the
introductory notes and details of the tar format.

Josh

Attachment Content-Type Size
pg_dump_README.diff application/octet-stream 3.1 KB

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Josh Kupershmidt <schmiddy(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: bad examples in pg_dump README
Date: 2013-01-05 14:34:26
Message-ID: CABUevEzqGi4b=ZKJE33-+ygYv9wT4v1_4eaC7PYNM+7Rknu_pg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Jan 4, 2013 at 3:36 AM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
> The ./src/bin/pg_dump README contains several inoperable examples.
> First, this suggestion:
>
> | or to list tables:
> |
> | pg_restore <backup-file> --table | less
>
> seems bogus, i.e. the --table option requires an argument specifing
> which table to restore, and does not "list tables". Next, this
> suggested command:
>
> | pg_restore <backup-file> -l --oid --rearrange | less
>
> hasn't worked since 7.4 or thereabouts, since we don't have
> --rearrange or --oid anymore. (I'm not sure we ever had --oid, maybe
> it was supposed to be --oid-order?). Then, three examples claim we
> support a "--use" flag, e.g.
>
> | pg_restore backup.bck --use=toc.lis > script.sql
>
> where presumably "--use-list" was meant. Further little gripes with
> this README include mixed use of tabs and spaces for the examples, and
> lines running over the 80-column limit which at least some of our
> other READMEs seem to honor.
>
> I started out attempting to fix up the README, keeping the original
> example commands intact, but upon further reflection I think the
> examples of dumping, restoring, and selective-restoring in that REAMDE
> don't belong there anyway. There are already better examples of such
> usage in the pg_dump/pg_restore documentation pages, and IMO that's
> where such generally-useful usage information belongs.
>
> I propose slimming down the pg_dump README, keeping intact the
> introductory notes and details of the tar format.

Do we need to keep it at all, really? Certainly the introductory part
is covered in the main documentation already...

I believe the tar notes are also out of date. For example, they claim
that you can't expect pg_restore to work with an uncompressed tar
format - yet the header in pg_backup_tar.c says that an uncompressed
tar format is compatible with a directory format dump, and thus you
*can* use pg_restore.

(And fwiw,the note about the user should probably go in src/port/ now
that we moved the tar header creation there a few days ago)

I would suggest we just drop the README file completely. I don't think
it adds any value at all.

Any objections to that path? :)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/


From: Josh Kupershmidt <schmiddy(at)gmail(dot)com>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: bad examples in pg_dump README
Date: 2013-01-05 18:21:49
Message-ID: CAK3UJRELooSnXj-5ReLW1C8yO_iSxp364rj-dpOWQAaXgN2TyQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sat, Jan 5, 2013 at 7:34 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Fri, Jan 4, 2013 at 3:36 AM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
> Do we need to keep it at all, really? Certainly the introductory part
> is covered in the main documentation already...

Pretty much. (I did find note #2 mildly interesting.)

> I believe the tar notes are also out of date. For example, they claim
> that you can't expect pg_restore to work with an uncompressed tar
> format - yet the header in pg_backup_tar.c says that an uncompressed
> tar format is compatible with a directory format dump, and thus you
> *can* use pg_restore.
>
> (And fwiw,the note about the user should probably go in src/port/ now
> that we moved the tar header creation there a few days ago)

Hrm yeah, so the second paragraph under the tar section can certainly be axed.

> I would suggest we just drop the README file completely. I don't think
> it adds any value at all.
>
> Any objections to that path? :)

I think that's OK, since there's not much left in that README after
removing the bogus examples, introductory text that's covered
elsewhere, and obsolete second paragraph about the tar format. Perhaps
we could keep the other paragraphs about the tar format, either in the
header comments for pg_backup_tar.c or in src/port/, though?

Oh, and for this comment in pg_backup_tar.c:

| * See the headers to pg_backup_files & pg_restore for more
details.

there is no longer a pg_backup_files.c.

Josh


From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Josh Kupershmidt <schmiddy(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: bad examples in pg_dump README
Date: 2013-01-08 03:12:22
Message-ID: 1357614742.19347.4.camel@vanquo.pezone.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sat, 2013-01-05 at 15:34 +0100, Magnus Hagander wrote:
> On Fri, Jan 4, 2013 at 3:36 AM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
> > I propose slimming down the pg_dump README, keeping intact the
> > introductory notes and details of the tar format.
>
> Do we need to keep it at all, really? Certainly the introductory part
> is covered in the main documentation already...

I'd remove it and distribute the remaining information, if any, between
the source code and the man page.


From: Josh Kupershmidt <schmiddy(at)gmail(dot)com>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: bad examples in pg_dump README
Date: 2013-01-09 02:55:07
Message-ID: CAK3UJRFqgYYT1Gqk=UrwwpRvu9_K3fv857_BdvO40VnfVHKH0g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Jan 7, 2013 at 8:12 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> On Sat, 2013-01-05 at 15:34 +0100, Magnus Hagander wrote:
>> On Fri, Jan 4, 2013 at 3:36 AM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
>> > I propose slimming down the pg_dump README, keeping intact the
>> > introductory notes and details of the tar format.
>>
>> Do we need to keep it at all, really? Certainly the introductory part
>> is covered in the main documentation already...
>
> I'd remove it and distribute the remaining information, if any, between
> the source code and the man page.

Here's a patch to do so. After removing the discussed bogus
information, there were only two notes which I still found relevant,
so I stuck those in the appropriate header comments.

I didn't preserve the comment about "blank username & group" for
tar'ed files, since there are already some comments along these lines
in tarCreateHeader(), and these are "postgres" not "blank" nowadays.
The pg_dump/pg_restore man pages seemed to already do a good enough
job of showing usage examples that I didn't find anything worth adding
there.

Josh

Attachment Content-Type Size
pg_dump_README.v2.diff application/octet-stream 3.8 KB

From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Josh Kupershmidt <schmiddy(at)gmail(dot)com>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: bad examples in pg_dump README
Date: 2013-01-17 04:52:31
Message-ID: 1358398351.21499.0.camel@vanquo.pezone.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, 2013-01-08 at 19:55 -0700, Josh Kupershmidt wrote:
> On Mon, Jan 7, 2013 at 8:12 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> > On Sat, 2013-01-05 at 15:34 +0100, Magnus Hagander wrote:
> >> On Fri, Jan 4, 2013 at 3:36 AM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
> >> > I propose slimming down the pg_dump README, keeping intact the
> >> > introductory notes and details of the tar format.
> >>
> >> Do we need to keep it at all, really? Certainly the introductory part
> >> is covered in the main documentation already...
> >
> > I'd remove it and distribute the remaining information, if any, between
> > the source code and the man page.
>
> Here's a patch to do so. After removing the discussed bogus
> information, there were only two notes which I still found relevant,
> so I stuck those in the appropriate header comments.

Committed.