Re: pg_dump reporing version of server & pg_dump as comments in the output - Mailing list pgsql-hackers

From Jeevan Chalke
Subject Re: pg_dump reporing version of server & pg_dump as comments in the output
Date
Msg-id CAM2+6=V4=eTq1RrUXZiVPSCGreVs3sD-Go7QNda=ETQUjR_jnw@mail.gmail.com
Whole thread Raw
In response to Re: pg_dump reporing version of server & pg_dump as comments in the output  ("Wang, Jing" <jingw@fast.au.fujitsu.com>)
Responses Re: pg_dump reporing version of server & pg_dump as comments in the output  (Abhijit Menon-Sen <ams@2ndQuadrant.com>)
Re: pg_dump reporing version of server & pg_dump as comments in the output  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Tue, Mar 4, 2014 at 11:28 AM, Wang, Jing <jingw@fast.au.fujitsu.com> wrote:
>I don't buy your argument. Why isn't verbose option sufficient? Did you
read the old thread about this [1]?
>[1] http://www.postgresql.org/message-id/3677.1253912361@sss.pgh.pa.us

>AFAICS a lot of people compare pg_dump diffs. If we apply this patch,
it would break those applications.

Well, as it already mentioned by Peter in above old mail thread that
"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"

So I don't think there will be any issue with application break.

>Also, it is *already* available if
you add verbose option (which is sufficient to satisfy those that want
the client and/or
>server version) in plain mode (the other modes already include the
desired info by default). In the past, timestamps were removed to avoid
noise in diffs.

Verbose option do include timestamps which will fail while comparing.
Also Verbose has too many details which may not be interested by people but I
guess more people will clearly wanted to see the server and client version.

Thus I see this patch is useful and thus reviewed it.


Sorry, I don't understand which application will break?  Can you give me
more detail information?
Timestamps always vary which do affect the diff.  But I can't imagine
why adding the version will affect the pg_dump diffs.
I don't think the version number vary  frequently.


Review comments:

1. Patch applies cleanly. make/make install/make check all are fine.
2. No issues fine as such in my unit testing.
3. As Tom suggested,
pg_dump > foo is giving identical output to pg_dump -Fc | pg_restore > foo

Basically only the code lines are re-arranged and thus it has NO affect on
system as such. So no issues found and as per me it is good to go in.

However following code chunk looks weird:

+   else
+   {
+       ahprintf(AH, "\n");
+   }


You need extra line when NOT verbose mode but you don't need that when it is
verbose mode. This is just because of the fact that dumpTimestamp() call in
verbose mode adds an extra line.
Can't we remove this else all-together and print extra line unconditionally ?
Also can we remove curly braces in if and else near these code changes ?
(Attached patch along those lines)

Anyone has any other views ?

Thanks

--
Jeevan B Chalke
Attachment

pgsql-hackers by date:

Previous
From: Hannu Krosing
Date:
Subject: Re: UPDATE SET (a,b,c) = (SELECT ...) versus rules
Next
From: Michael Paquier
Date:
Subject: Re: IMPORT FOREIGN SCHEMA statement