Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server

Lists: pgsql-hackers
From: "shakahshakah(at)gmail(dot)com" <shakahshakah(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-09-25 18:52:16
Message-ID: 1cfaef52-a45e-4199-8244-114806cec685@o21g2000vbl.googlegroups.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

From pg_dump/pg_restore section (9.2 of the Todo page on the
PostgreSQL Wiki), is the following item
"Add comments to output indicating version of pg_dump and of the
database server"

simply asking for a change to the pg_dump header from:

--
-- PostgreSQL database dump
--

to something like:

-- PostgreSQL database dump
--
-- pg_dump version: 8.5devel
--
-- remote database version: 8.5devel (80500)
--

?


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "shakahshakah(at)gmail(dot)com" <shakahshakah(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-09-25 20:59:21
Message-ID: 3677.1253912361@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

"shakahshakah(at)gmail(dot)com" <shakahshakah(at)gmail(dot)com> writes:
> From pg_dump/pg_restore section (9.2 of the Todo page on the
> PostgreSQL Wiki), is the following item
> "Add comments to output indicating version of pg_dump and of the
> database server"
> simply asking for a change to the pg_dump header from:

I think so, but what's not clear is whether this is a good idea to do
in the default output. It might only be appropriate in "verbose" mode,
so as not to introduce unnecessary diffs between logically identical
dumps. We long ago got rid of timestamps in the default output for
exactly that reason.

Another issue is that it's not all that clear what to do or how to do it
for archive dumps --- do you then want both pg_dump and pg_restore to
tell you about themselves? If pg_restore adds anything, then this'd
also break the principle that pg_dump >foo should give identical output
to pg_dump -Fc | pg_restore >foo. Which is something that I for one
put a great deal of stock in, for testing purposes.

regards, tom lane


From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "shakahshakah(at)gmail(dot)com" <shakahshakah(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-09-26 20:02:55
Message-ID: 1253995375.2880.5.camel@vanquo.pezone.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, 2009-09-25 at 16:59 -0400, Tom Lane wrote:
> "shakahshakah(at)gmail(dot)com" <shakahshakah(at)gmail(dot)com> writes:
> > From pg_dump/pg_restore section (9.2 of the Todo page on the
> > PostgreSQL Wiki), is the following item
> > "Add comments to output indicating version of pg_dump and of the
> > database server"
> > simply asking for a change to the pg_dump header from:
>
> I think so, but what's not clear is whether this is a good idea to do
> in the default output. It might only be appropriate in "verbose" mode,
> so as not to introduce unnecessary diffs between logically identical
> dumps.

Well, a diff of the same database made by different (major) versions of
pg_dump will already be different in most situations, so adding the
pg_dump version number in it is essentially free from this perspective.

What is the use case for adding the server version?

I can imagine something like wanting to know exactly where the dump came
from, but then host name and such would be better. (And then you can
infer the server version from that.)

> Another issue is that it's not all that clear what to do or how to do it
> for archive dumps --- do you then want both pg_dump and pg_restore to
> tell you about themselves?

I don't see a good reason for pg_restore to get involved.


From: David Fetter <david(at)fetter(dot)org>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "shakahshakah(at)gmail(dot)com" <shakahshakah(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-09-26 22:48:43
Message-ID: 20090926224843.GD7952@fetter.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sat, Sep 26, 2009 at 11:02:55PM +0300, Peter Eisentraut wrote:
> On Fri, 2009-09-25 at 16:59 -0400, Tom Lane wrote:
> > "shakahshakah(at)gmail(dot)com" <shakahshakah(at)gmail(dot)com> writes:
> > > From pg_dump/pg_restore section (9.2 of the Todo page on the
> > > PostgreSQL Wiki), is the following item "Add comments to output
> > > indicating version of pg_dump and of the database server" simply
> > > asking for a change to the pg_dump header from:
> >
> > I think so, but what's not clear is whether this is a good idea to
> > do in the default output. It might only be appropriate in
> > "verbose" mode, so as not to introduce unnecessary diffs between
> > logically identical dumps.
>
> Well, a diff of the same database made by different (major) versions
> of pg_dump will already be different in most situations, so adding
> the pg_dump version number in it is essentially free from this
> perspective.
>
> What is the use case for adding the server version?

There have been cases where pg_restore doesn't fix infelicities. For
example, there was a time when it was a good idea to run adddepend
after the reload. Knowing what server version the dump came from
could be handy for this kind of case.

> I can imagine something like wanting to know exactly where the dump
> came from, but then host name and such would be better. (And then
> you can infer the server version from that.)

You can infer the server version until the next upgrade, at which
point the information is lost.

Cheers,
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


From: Jim Cox <shakahshakah(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-09-27 11:32:41
Message-ID: c2ee6dbd0909270432hd7773edk144080185fb5259d@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sat, Sep 26, 2009 at 6:48 PM, David Fetter <david(at)fetter(dot)org> wrote:
> On Sat, Sep 26, 2009 at 11:02:55PM +0300, Peter Eisentraut wrote:
>> On Fri, 2009-09-25 at 16:59 -0400, Tom Lane wrote:
>> > "shakahshakah(at)gmail(dot)com" <shakahshakah(at)gmail(dot)com> writes:
>> > > From pg_dump/pg_restore section (9.2 of the Todo page on the
>> > > PostgreSQL Wiki), is the following item "Add comments to output
>> > > indicating version of pg_dump and of the database server" simply
>> > > asking for a change to the pg_dump header from:
>> >
>> > I think so, but what's not clear is whether this is a good idea to
>> > do in the default output.  It might only be appropriate in
>> > "verbose" mode, so as not to introduce unnecessary diffs between
>> > logically identical dumps.
>>
>> Well, a diff of the same database made by different (major) versions
>> of pg_dump will already be different in most situations, so adding
>> the pg_dump version number in it is essentially free from this
>> perspective.
>>
>> What is the use case for adding the server version?
>
> There have been cases where pg_restore doesn't fix infelicities.  For
> example, there was a time when it was a good idea to run adddepend
> after the reload.  Knowing what server version the dump came from
> could be handy for this kind of case.
>
>> I can imagine something like wanting to know exactly where the dump
>> came from, but then host name and such would be better.  (And then
>> you can infer the server version from that.)
>
> You can infer the server version until the next upgrade, at which
> point the information is lost.
>
> Cheers,
> David.

Attached s/b a patch for the 8.5 TODO "Add comments to output indicating version
of pg_dump and of the database server" (pg_dump/pg_restore section, 9.2).

In verbose mode, pg_dump version and remote database version now appear
in the "plain" output format.

For example, original verbose output:
--
-- PostgreSQL database dump
--

-- Started on 2009-09-27 11:05:52 UTC

SET statement_timeout = 0;
[...etc...]

New verbose output:
--
-- PostgreSQL database dump
--

--
-- pg_dump version: 8.5devel
--
-- remote database version: 8.1.3 (80103)
--

-- Started on 2009-09-27 11:05:52 UTC

SET statement_timeout = 0;
[...etc...]

Attachment Content-Type Size
add_pgdump_and_db_versions_v1.patch text/x-patch 858 bytes

From: Chris Browne <cbbrowne(at)acm(dot)org>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-09-28 16:55:32
Message-ID: 87fxa79fgb.fsf@dba2.int.libertyrms.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

peter_e(at)gmx(dot)net (Peter Eisentraut) writes:
> On Fri, 2009-09-25 at 16:59 -0400, Tom Lane wrote:
>> "shakahshakah(at)gmail(dot)com" <shakahshakah(at)gmail(dot)com> writes:
>> > From pg_dump/pg_restore section (9.2 of the Todo page on the
>> > PostgreSQL Wiki), is the following item
>> > "Add comments to output indicating version of pg_dump and of the
>> > database server"
>> > simply asking for a change to the pg_dump header from:
>>
>> I think so, but what's not clear is whether this is a good idea to do
>> in the default output. It might only be appropriate in "verbose" mode,
>> so as not to introduce unnecessary diffs between logically identical
>> dumps.
>
> Well, a diff of the same database made by different (major) versions of
> pg_dump will already be different in most situations, so adding the
> pg_dump version number in it is essentially free from this perspective.
>
> What is the use case for adding the server version?
>
> I can imagine something like wanting to know exactly where the dump came
> from, but then host name and such would be better. (And then you can
> infer the server version from that.)

I added this ToDo because we had a case where we were spelunking
through some old pg_dumps, and the provenance was sufficiently distant
that we couldn't readily infer what PostgreSQL version was involved.

If pg_dump reported something like:

-- pg_dump version: 8.5_devel
-- postgres server version: 8.4.17

then it would be trivial to ascertain the information.

Actually, I have no argument with your point; perhaps a whole "header
section" is the right answer:

-- pg_dump version: 8.5_devel
-- postgres server version: 8.4.17
-- dump began at: 2010-07-01 14:22:27 EDT
-- server name: wolfe
-- more, maybe?

>> Another issue is that it's not all that clear what to do or how to do it
>> for archive dumps --- do you then want both pg_dump and pg_restore to
>> tell you about themselves?
>
> I don't see a good reason for pg_restore to get involved.

Agreed. This isn't needed for pg_restore to do anything better; it's
so that humans can do better "archaeology."
--
let name="cbbrowne" and tld="acm.org" in name ^ "@" ^ tld;;
http://linuxfinances.info/info/languages.html
Rules of the Evil Overlord #187. "I will not hold lavish banquets in
the middle of a famine. The good PR among the guests doesn't make up
for the bad PR among the masses." <http://www.eviloverlord.com/>


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Jim Cox <shakahshakah(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-09-29 04:00:38
Message-ID: 20090929040038.GA6770@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Jim Cox escribió:

> Attached s/b a patch for the 8.5 TODO "Add comments to output indicating version
> of pg_dump and of the database server" (pg_dump/pg_restore section, 9.2).

Hmm, what happens if you do a pg_dump -Fc? Is this info saved anywhere
in the dump? Surely if thi is useful in the text dump, it is useful in
the binary format dumps too.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


From: Jim Cox <shakahshakah(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-09-29 13:58:08
Message-ID: c2ee6dbd0909290658k974a80axb8cbed26189f3c88@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Sep 29, 2009 at 12:00 AM, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:
> Jim Cox escribió:
>
>> Attached s/b a patch for the 8.5 TODO "Add comments to output indicating version
>> of pg_dump and of the database server" (pg_dump/pg_restore section, 9.2).
>
> Hmm, what happens if you do a pg_dump -Fc?  Is this info saved anywhere
> in the dump?  Surely if thi is useful in the text dump, it is useful in
> the binary format dumps too.

(forgot to reply all)

pg_restore's "-l, --list print summarized TOC of the archive" option
does display the information in custom format dumps, e.g.:

prompt$ /usr/local/pgsql/bin/pg_restore -l < /tmp/mytest.dump
;
; Archive created at Tue Sep 29 13:48:37 2009
; dbname: mytest
; TOC Entries: 9
; Compression: -1
; Dump Version: 1.11-0
; Format: CUSTOM
; Integer: 4 bytes
; Offset: 8 bytes
; Dumped from database version: 8.4.0
; Dumped by pg_dump version: 8.4.0


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Jim Cox <shakahshakah(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-10-02 22:07:33
Message-ID: 200910022207.n92M7Xc29131@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Jim Cox wrote:
> On Tue, Sep 29, 2009 at 12:00 AM, Alvaro Herrera
> <alvherre(at)commandprompt(dot)com> wrote:
> > Jim Cox escribi?:
> >
> >> Attached s/b a patch for the 8.5 TODO "Add comments to output indicating version
> >> of pg_dump and of the database server" (pg_dump/pg_restore section, 9.2).
> >
> > Hmm, what happens if you do a pg_dump -Fc? ?Is this info saved anywhere
> > in the dump? ?Surely if thi is useful in the text dump, it is useful in
> > the binary format dumps too.
>
> (forgot to reply all)
>
> pg_restore's "-l, --list print summarized TOC of the archive" option
> does display the information in custom format dumps, e.g.:
>
> prompt$ /usr/local/pgsql/bin/pg_restore -l < /tmp/mytest.dump
> ;
> ; Archive created at Tue Sep 29 13:48:37 2009
> ; dbname: mytest
> ; TOC Entries: 9
> ; Compression: -1
> ; Dump Version: 1.11-0
> ; Format: CUSTOM
> ; Integer: 4 bytes
> ; Offset: 8 bytes
> ; Dumped from database version: 8.4.0
> ; Dumped by pg_dump version: 8.4.0

Are we sure we don't want a date/time in the ASCII dump? It would
affect database diffs, but I can see it useful too.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Jim Cox <shakahshakah(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-10-02 22:10:58
Message-ID: 15271.1254521458@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Are we sure we don't want a date/time in the ASCII dump? It would
> affect database diffs, but I can see it useful too.

We used to have one, and it was removed by popular demand
(or actually demoted to be printed only in verbose mode, IIRC).
I don't feel a need to revisit that decision.

regards, tom lane


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jim Cox <shakahshakah(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-10-02 22:11:48
Message-ID: 200910022211.n92MBmP01193@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Are we sure we don't want a date/time in the ASCII dump? It would
> > affect database diffs, but I can see it useful too.
>
> We used to have one, and it was removed by popular demand
> (or actually demoted to be printed only in verbose mode, IIRC).
> I don't feel a need to revisit that decision.

OK, thanks, I had forgotten.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +


From: Jim Cox <shakahshakah(at)gmail(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-10-02 22:16:20
Message-ID: c2ee6dbd0910021516x47dc749bqab37fa5cf5cc1262@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Oct 2, 2009 at 6:11 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Tom Lane wrote:
>> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> > Are we sure we don't want a date/time in the ASCII dump?  It would
>> > affect database diffs, but I can see it useful too.
>>
>> We used to have one, and it was removed by popular demand
>> (or actually demoted to be printed only in verbose mode, IIRC).
>> I don't feel a need to revisit that decision.
>
> OK, thanks, I had forgotten.
>

To Tom's point, the patch only outputs the new info in verbose mode,
and in that mode a timestamp is indeed already output (it is the
subsequent line in the ASCII dump, actually).


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Jim Cox <shakahshakah(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-10-02 22:18:56
Message-ID: 200910022218.n92MIuQ01982@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Jim Cox wrote:
> On Fri, Oct 2, 2009 at 6:11 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > Tom Lane wrote:
> >> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> >> > Are we sure we don't want a date/time in the ASCII dump? ?It would
> >> > affect database diffs, but I can see it useful too.
> >>
> >> We used to have one, and it was removed by popular demand
> >> (or actually demoted to be printed only in verbose mode, IIRC).
> >> I don't feel a need to revisit that decision.
> >
> > OK, thanks, I had forgotten.
> >
>
> To Tom's point, the patch only outputs the new info in verbose mode,
> and in that mode a timestamp is indeed already output (it is the
> subsequent line in the ASCII dump, actually).

Thank you.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +


From: Philip Warner <pjw(at)rhyme(dot)com(dot)au>
To: "shakahshakah(at)gmail(dot)com" <shakahshakah(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2009-11-29 01:38:06
Message-ID: 4B11D07E.6050407@rhyme.com.au
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

shakahshakah(at)gmail(dot)com wrote:
> -- PostgreSQL database dump
> --
> -- pg_dump version: 8.5devel
> --
> -- remote database version: 8.5devel (80500)
> --
>

FWIW, and I havent read the entire thread, but pg_dump already *stores*
this information in a custom format. Try:

pg_dump -Fc blah
pg_restore -L ...

and you will get something like:

;
; Archive created at Sun Nov 29 12:34:24 2009
; dbname: blah
; TOC Entries: 202
; Compression: -1
; Dump Version: 1.10-0
; Format: CUSTOM
; Integer: 4 bytes
; Offset: 8 bytes
; Dumped from database version: 8.0.3
; Dumped by pg_dump version: 8.0.3
;

so, all that is needed is to add the relevant statements into the output
code.


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Jim Cox <shakahshakah(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-23 01:55:34
Message-ID: 201002230155.o1N1tYX08601@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


What happened to this patch? I don't see any objections, but it was not
applied.

---------------------------------------------------------------------------

Jim Cox wrote:
> On Tue, Sep 29, 2009 at 12:00 AM, Alvaro Herrera
> <alvherre(at)commandprompt(dot)com> wrote:
> > Jim Cox escribi?:
> >
> >> Attached s/b a patch for the 8.5 TODO "Add comments to output indicating version
> >> of pg_dump and of the database server" (pg_dump/pg_restore section, 9.2).
> >
> > Hmm, what happens if you do a pg_dump -Fc? ?Is this info saved anywhere
> > in the dump? ?Surely if thi is useful in the text dump, it is useful in
> > the binary format dumps too.
>
> (forgot to reply all)
>
> pg_restore's "-l, --list print summarized TOC of the archive" option
> does display the information in custom format dumps, e.g.:
>
> prompt$ /usr/local/pgsql/bin/pg_restore -l < /tmp/mytest.dump
> ;
> ; Archive created at Tue Sep 29 13:48:37 2009
> ; dbname: mytest
> ; TOC Entries: 9
> ; Compression: -1
> ; Dump Version: 1.11-0
> ; Format: CUSTOM
> ; Integer: 4 bytes
> ; Offset: 8 bytes
> ; Dumped from database version: 8.4.0
> ; Dumped by pg_dump version: 8.4.0
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
+ If your life is a hard drive, Christ can be your backup. +


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Jim Cox <shakahshakah(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-23 18:14:38
Message-ID: 603c8f071002231014o66c3542cuadd3e52cd5fda087@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Feb 22, 2010 at 8:55 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> What happened to this patch? I don't see any objections, but it was not
> applied.

I think that the patch author never added it to the open CommitFest
and nobody else thought it was important enough to pick up. It looks
innocuous to me; want to go ahead and apply?

...Robert


From: Josh Berkus <josh(at)agliodbs(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Jim Cox <shakahshakah(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-23 18:21:02
Message-ID: 4B841C8E.5000102@agliodbs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2/23/10 10:14 AM, Robert Haas wrote:
> On Mon, Feb 22, 2010 at 8:55 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> What happened to this patch? I don't see any objections, but it was not
>> applied.
>
> I think that the patch author never added it to the open CommitFest
> and nobody else thought it was important enough to pick up. It looks
> innocuous to me; want to go ahead and apply?

I'd say yes. It's post-freeze, but this falls into the class of "oops,
we forgot about this patch" which the CFs were designed to prevent.

--Josh Berkus


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Josh Berkus <josh(at)agliodbs(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Jim Cox <shakahshakah(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-23 18:26:29
Message-ID: 27121.1266949589@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> On 2/23/10 10:14 AM, Robert Haas wrote:
>> On Mon, Feb 22, 2010 at 8:55 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>>> What happened to this patch? I don't see any objections, but it was not
>>> applied.
>>
>> I think that the patch author never added it to the open CommitFest
>> and nobody else thought it was important enough to pick up. It looks
>> innocuous to me; want to go ahead and apply?

> I'd say yes. It's post-freeze, but this falls into the class of "oops,
> we forgot about this patch" which the CFs were designed to prevent.

That would be an argument for sticking this in the next CF, not for
applying it now --- it was submitted after the close of the last CF no?

regards, tom lane


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Josh Berkus <josh(at)agliodbs(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Jim Cox <shakahshakah(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-23 18:29:43
Message-ID: 20100223182943.GL3672@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane escribió:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
> > On 2/23/10 10:14 AM, Robert Haas wrote:
> >> On Mon, Feb 22, 2010 at 8:55 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >>> What happened to this patch? I don't see any objections, but it was not
> >>> applied.
> >>
> >> I think that the patch author never added it to the open CommitFest
> >> and nobody else thought it was important enough to pick up. It looks
> >> innocuous to me; want to go ahead and apply?
>
> > I'd say yes. It's post-freeze, but this falls into the class of "oops,
> > we forgot about this patch" which the CFs were designed to prevent.
>
> That would be an argument for sticking this in the next CF, not for
> applying it now --- it was submitted after the close of the last CF no?

Sep. 29 2009?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Josh Berkus <josh(at)agliodbs(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Jim Cox <shakahshakah(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-23 18:47:57
Message-ID: 125.1266950877@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Tom Lane escribi:
>> That would be an argument for sticking this in the next CF, not for
>> applying it now --- it was submitted after the close of the last CF no?

> Sep. 29 2009?

Oh, I was thinking it had just come in recently, but looking back you're
right. It did slip through the cracks.

However, has the patch actually been reviewed? pg_dump is a piece of
code where it is notoriously easy for novices to do things wrong,
and this is especially true for adding output that should only come out
in particular cases.

regards, tom lane


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Josh Berkus <josh(at)agliodbs(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Jim Cox <shakahshakah(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-23 19:19:51
Message-ID: 603c8f071002231119u3fcddf20p1cb0abf7e47b44dc@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Feb 23, 2010 at 1:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>> Tom Lane escribió:
>>> That would be an argument for sticking this in the next CF, not for
>>> applying it now --- it was submitted after the close of the last CF no?
>
>> Sep. 29 2009?
>
> Oh, I was thinking it had just come in recently, but looking back you're
> right.  It did slip through the cracks.
>
> However, has the patch actually been reviewed?  pg_dump is a piece of
> code where it is notoriously easy for novices to do things wrong,
> and this is especially true for adding output that should only come out
> in particular cases.

It's a fairly trivial patch. I took a quick look at it. It needs
more than that, but I think not too much more. I think it would be
less effort for someone to review it and make a decision than it would
be to keep it as an open item for the next 6 months. But that's just
MHO: if the consensus is to postpone it, then let's just do that and
move on.

...Robert


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Josh Berkus <josh(at)agliodbs(dot)com>, Jim Cox <shakahshakah(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-23 21:49:32
Message-ID: 201002232149.o1NLnWN13937@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Robert Haas wrote:
> On Tue, Feb 23, 2010 at 1:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> >> Tom Lane escribi?:
> >>> That would be an argument for sticking this in the next CF, not for
> >>> applying it now --- it was submitted after the close of the last CF no?
> >
> >> Sep. 29 2009?
> >
> > Oh, I was thinking it had just come in recently, but looking back you're
> > right. ?It did slip through the cracks.
> >
> > However, has the patch actually been reviewed? ?pg_dump is a piece of
> > code where it is notoriously easy for novices to do things wrong,
> > and this is especially true for adding output that should only come out
> > in particular cases.
>
> It's a fairly trivial patch. I took a quick look at it. It needs
> more than that, but I think not too much more. I think it would be
> less effort for someone to review it and make a decision than it would
> be to keep it as an open item for the next 6 months.

Agreed, applied, and TODO updated.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
+ If your life is a hard drive, Christ can be your backup. +


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Josh Berkus <josh(at)agliodbs(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Jim Cox <shakahshakah(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-23 22:12:44
Message-ID: 16950.1266963164@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Feb 23, 2010 at 1:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> However, has the patch actually been reviewed? pg_dump is a piece of
>> code where it is notoriously easy for novices to do things wrong,
>> and this is especially true for adding output that should only come out
>> in particular cases.

> It's a fairly trivial patch. I took a quick look at it. It needs
> more than that, but I think not too much more. I think it would be
> less effort for someone to review it and make a decision than it would
> be to keep it as an open item for the next 6 months. But that's just
> MHO: if the consensus is to postpone it, then let's just do that and
> move on.

Well, "trivial" and "correct" are entirely different things :-(.
If we're still talking about
http://archives.postgresql.org/message-id/c2ee6dbd0909270432hd7773edk144080185fb5259d@mail.gmail.com
then it is in fact printing the wrong thing for pg_dump's version.
PG_VERSION is a compiled-in constant so what you will get when examining
an archive is pg_restore's version not pg_dump's version. This is
no doubt fixable but it looks like the code doesn't currently bother
to set archiveDumpVersion in the plain pg_dump code path, so it's
not entirely trivial.

regards, tom lane


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Josh Berkus <josh(at)agliodbs(dot)com>, Jim Cox <shakahshakah(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-23 22:27:00
Message-ID: 201002232227.o1NMR0c23595@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Tue, Feb 23, 2010 at 1:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> However, has the patch actually been reviewed? pg_dump is a piece of
> >> code where it is notoriously easy for novices to do things wrong,
> >> and this is especially true for adding output that should only come out
> >> in particular cases.
>
> > It's a fairly trivial patch. I took a quick look at it. It needs
> > more than that, but I think not too much more. I think it would be
> > less effort for someone to review it and make a decision than it would
> > be to keep it as an open item for the next 6 months. But that's just
> > MHO: if the consensus is to postpone it, then let's just do that and
> > move on.
>
> Well, "trivial" and "correct" are entirely different things :-(.
> If we're still talking about
> http://archives.postgresql.org/message-id/c2ee6dbd0909270432hd7773edk144080185fb5259d@mail.gmail.com

Yes, that is the patch.

> then it is in fact printing the wrong thing for pg_dump's version.

Uh, right now it is printing:

-- pg_dump version: 9.0devel
--
-- remote database version: 9.0devel (90000)

That is in the SQL output file.

> PG_VERSION is a compiled-in constant so what you will get when examining
> an archive is pg_restore's version not pg_dump's version. This is
> no doubt fixable but it looks like the code doesn't currently bother
> to set archiveDumpVersion in the plain pg_dump code path, so it's
> not entirely trivial.

So you are saying if you run pg_restore on the SQL dump file, it doesn't
pick up the version? I didn't even know pg_restore could do that for
text dump files. In fact, I can't get it to work:

$ pg_dump -v test > /rtmp/x
...
$ pg_restore -l /rtmp/x -d test
pg_restore: [archiver] input file does not appear to be a valid archive

I obviously am missing something.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
+ If your life is a hard drive, Christ can be your backup. +


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Josh Berkus <josh(at)agliodbs(dot)com>, Jim Cox <shakahshakah(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Peter Eisentraut <peter_e(at)gmx(dot)net>, David Fetter <david(at)fetter(dot)org>
Subject: Re: [PATCH] 8.5 TODO: Add comments to output indicating version of pg_dump and of the database server
Date: 2010-02-24 02:46:14
Message-ID: 22696.1266979574@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> Well, "trivial" and "correct" are entirely different things :-(.

> So you are saying if you run pg_restore on the SQL dump file, it doesn't
> pick up the version?

No, I'm saying it printed the wrong thing if you did pg_restore -v from
an archive file made by a different pg_dump version. I fixed it, but
this patch was *not* ready to commit.

regards, tom lane