Thread: pg_dump reporing version of server & pg_dump as comments in the output

pg_dump reporing version of server & pg_dump as comments in the output

From

"Wang, Jing"

Date:

28 February 2014, 03:11:15

<div class="WordSection1"><p class="MsoNormal">Enclosed is the patch to implement the requirement that pg_dump should
reportversion of server & pg_dump as comments in the output. <p class="MsoNormal"> <p class="MsoNormal">[Benefit]<p
class="MsoNormal">Byrunning "head" on pg_dump output, you can readily discover what version of PostgreSQL was used to
generatethat dump. Very useful especially for "mouldy old database dumps."<p class="MsoNormal">The benefit of this
requirementis to let user clearly understand from which version the dump output file will be insert into which version
databaseserver and easy handle the problems of Incompatibility versions.<p class="MsoNormal"> <p
class="MsoNormal">[Analysis]<pclass="MsoNormal">Using pg_dump can dump the data into the file with format set to be
'c','t'or plain text. In the existing version the version of server & pg_dump is already there when the format of
fileis 'c' or 't'. And even for the plain text format file the version of server & pg_dump is already there if
using'--verbose' in pg_dump. Using '--verbose' leads to some many other prints which are not required always. <p
class="MsoNormal"> <pclass="MsoNormal">So the requirement is dump the version of server & pg_dump as comment into
theplain text format output file even without using '--verbose' option.<p class="MsoNormal"> <p
class="MsoNormal">[Solutiondetails]<p class="MsoNormal">The main change is in the pg_backup_archiver.c file, in the
function'RestoreArchive' the version of server & pg_dump is only print when finding the '--verbose' option to be
usedin current version.  Now we just let the printing works even without finding the '--verbose' option.<p
class="MsoNormal"> <pclass="MsoNormal">[what is changed when applying the patch]<p class="MsoNormal">1. The output file
whichis created by pg_dump with format set to be 'plain text' and without using ‘--verbose’ option will include the
versionof server & pg_dump. One example is  as following:<p class="MsoNormal">--<p class="MsoNormal">-- PostgreSQL
databasedump<p class="MsoNormal">--<p class="MsoNormal"> <p class="MsoNormal">-- Dumped from database version 9.2.4<p
class="MsoNormal">--Dumped by pg_dump version 9.4devel<p class="MsoNormal"> <p class="MsoNormal">SET statement_timeout
=0;<p class="MsoNormal">SET lock_timeout = 0;<p class="MsoNormal">SET client_encoding = 'UTF8';<p
class="MsoNormal">...<pclass="MsoNormal"> <p class="MsoNormal">2. The output file which is created by pg_dumpall with
formatset to be 'plain text' and without using ‘--verbose’ option will include the version of server & pg_dump. The
exampleis as following:<p class="MsoNormal"> <p class="MsoNormal">--<p class="MsoNormal">-- PostgreSQL database cluster
dump<pclass="MsoNormal">--<p class="MsoNormal"> <p class="MsoNormal">SET default_transaction_read_only = off;<p
class="MsoNormal"> <pclass="MsoNormal">...<p class="MsoNormal"> <p class="MsoNormal">\connect connectdb<p
class="MsoNormal"> <pclass="MsoNormal">SET default_transaction_read_only = off;<p class="MsoNormal"> <p
class="MsoNormal">--<pclass="MsoNormal">-- PostgreSQL database dump<p class="MsoNormal">--<p class="MsoNormal"> <p
class="MsoNormal">--Dumped from database version 9.2.4<p class="MsoNormal">-- Dumped by pg_dump version 9.4devel<p
class="MsoNormal"> <pclass="MsoNormal">SET statement_timeout = 0;<p class="MsoNormal">SET lock_timeout = 0;<p
class="MsoNormal">SETclient_encoding = 'UTF8';<p class="MsoNormal">SET standard_conforming_strings = on;<p
class="MsoNormal"> <pclass="MsoNormal">...<p class="MsoNormal"> <p class="MsoNormal">\connect postgres<p
class="MsoNormal"> <pclass="MsoNormal">SET default_transaction_read_only = off;<p class="MsoNormal"> <p
class="MsoNormal">--<pclass="MsoNormal">-- PostgreSQL database dump<p class="MsoNormal">--<p class="MsoNormal"> <p
class="MsoNormal">--Dumped from database version 9.2.4<p class="MsoNormal">-- Dumped by pg_dump version 9.4devel<p
class="MsoNormal"> <pclass="MsoNormal">SET statement_timeout = 0;<p class="MsoNormal">SET lock_timeout = 0;<p
class="MsoNormal">SETclient_encoding = 'UTF8';<p class="MsoNormal">SET standard_conforming_strings = on;<p
class="MsoNormal">SETcheck_function_bodies = false;<p class="MsoNormal"> <p class="MsoNormal"> <p class="MsoNormal">3.
Theversion of server and pg_dump will be dumped into the output file. The output file is created by the following
command:<pclass="MsoNormal">pg_restore inputFile -f output.sql <p class="MsoNormal"> <p class="MsoNormal">One example
isas following:<p class="MsoNormal">--<p class="MsoNormal">-- PostgreSQL database dump<p class="MsoNormal">--<p
class="MsoNormal"> <pclass="MsoNormal">-- Dumped from database version 9.2.4<p class="MsoNormal">-- Dumped by pg_dump
version9.4devel<p class="MsoNormal"> <p class="MsoNormal">SET statement_timeout = 0;<p class="MsoNormal">SET
lock_timeout= 0;<p class="MsoNormal">SET client_encoding = 'UTF8';<p class="MsoNormal">SET standard_conforming_strings
=on;<p class="MsoNormal">…<p class="MsoNormal"> <p class="MsoNormal"> <p class="MsoNormal">Kind regards<p
class="MsoNormal">Jing</div>

Re: pg_dump reporing version of server & pg_dump as comments in the output

From

"Wang, Jing"

Date:

28 February 2014, 03:29:17

Sorry for missing the patch file in the original email. Enclosed please find it.

Jing Wang

Fujitsu Australia

From: Arulappan, Arul Shaji
Sent: Friday, 28 February 2014 11:21 AM
To: Wang, Jing
Subject: RE: [HACKERS] pg_dump reporing version of server & pg_dump as comments in the output
Importance: High

Jing, You missed the patch attachement.

Rgds,

Arul Shaji

From: pgsql-hackers-owner@postgresql.org [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Wang, Jing
Sent: Friday, 28 February 2014 11:11 AM
To: pgsql-hackers@postgresql.org
Subject: [HACKERS] pg_dump reporing version of server & pg_dump as comments in the output

Enclosed is the patch to implement the requirement that pg_dump should report version of server & pg_dump as comments in the output.

[Benefit]

By running "head" on pg_dump output, you can readily discover what version of PostgreSQL was used to generate that dump. Very useful especially for "mouldy old database dumps."

The benefit of this requirement is to let user clearly understand from which version the dump output file will be insert into which version database server and easy handle the problems of Incompatibility versions.

[Analysis]

Using pg_dump can dump the data into the file with format set to be 'c','t' or plain text. In the existing version the version of server & pg_dump is already there when the format of file is 'c' or 't'. And even for the plain text format file the version of server & pg_dump is already there if using '--verbose' in pg_dump. Using '--verbose' leads to some many other prints which are not required always.

So the requirement is dump the version of server & pg_dump as comment into the plain text format output file even without using '--verbose' option.

[Solution details]

The main change is in the pg_backup_archiver.c file, in the function 'RestoreArchive' the version of server & pg_dump is only print when finding the '--verbose' option to be used in current version. Now we just let the printing works even without finding the '--verbose' option.

[what is changed when applying the patch]

1. The output file which is created by pg_dump with format set to be 'plain text' and without using ‘--verbose’ option will include the version of server & pg_dump. One example is as following:

-- PostgreSQL database dump

-- Dumped from database version 9.2.4

-- Dumped by pg_dump version 9.4devel

SET statement_timeout = 0;

SET lock_timeout = 0;

SET client_encoding = 'UTF8';

...

2. The output file which is created by pg_dumpall with format set to be 'plain text' and without using ‘--verbose’ option will include the version of server & pg_dump. The example is as following:

-- PostgreSQL database cluster dump

SET default_transaction_read_only = off;

...

\connect connectdb

SET default_transaction_read_only = off;

-- PostgreSQL database dump

-- Dumped from database version 9.2.4

-- Dumped by pg_dump version 9.4devel

SET statement_timeout = 0;

SET lock_timeout = 0;

SET client_encoding = 'UTF8';

SET standard_conforming_strings = on;

...

\connect postgres

SET default_transaction_read_only = off;

-- PostgreSQL database dump

-- Dumped from database version 9.2.4

-- Dumped by pg_dump version 9.4devel

SET statement_timeout = 0;

SET lock_timeout = 0;

SET client_encoding = 'UTF8';

SET standard_conforming_strings = on;

SET check_function_bodies = false;

3. The version of server and pg_dump will be dumped into the output file. The output file is created by the following command:

pg_restore inputFile -f output.sql

One example is as following:

-- PostgreSQL database dump

-- Dumped from database version 9.2.4

-- Dumped by pg_dump version 9.4devel

SET statement_timeout = 0;

SET lock_timeout = 0;

SET client_encoding = 'UTF8';

SET standard_conforming_strings = on;

…

Kind regards

Jing

Attachment

pg_dump.patch

Re: pg_dump reporing version of server & pg_dump as comments in the output

From

Euler Taveira

Date:

03 March 2014, 18:41:26

On 27-02-2014 21:10, Wang, Jing wrote:
> Using pg_dump can dump the data into the file with format set to be
> 'c','t' or plain text. In the existing version the version of server &
> pg_dump is already there when the format of file is 'c' or 't'. And even
> for the plain text format file the version of server & pg_dump is
> already there if using '--verbose' in pg_dump. Using '--verbose' leads
> to some many other prints which are not required always. 
> 
I don't buy your argument. Why isn't verbose option sufficient? Did you
read the old thread about this [1]?

AFAICS a lot of people compare pg_dump diffs. If we apply this patch, it
would break those applications. 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.

[1] http://www.postgresql.org/message-id/3677.1253912361@sss.pgh.pa.us

--   Euler Taveira                   Timbira - http://www.timbira.com.br/  PostgreSQL: Consultoria, Desenvolvimento,
Suporte24x7 e Treinamento

Re: pg_dump reporing version of server & pg_dump as comments in the output

From

"Wang, Jing"

Date:

04 March 2014, 08:58:41

On 4 March 2014 2:41 Euler Taveira wrote:
>On 27-02-2014 21:10, Wang, Jing wrote:
>> Using pg_dump can dump the data into the file with format set to be
>> 'c','t' or plain text. In the existing version the version of server
&
>> pg_dump is already there when the format of file is 'c' or 't'. And
>> even for the plain text format file the version of server & pg_dump
is
>> already there if using '--verbose' in pg_dump. Using '--verbose'
leads
>> to some many other prints which are not required always.
>>
>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. 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.

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.

Kind regards
Jing Wang
Fujitsu Australia

Re: pg_dump reporing version of server & pg_dump as comments in the output

From

Jeevan Chalke

Date:

17 June 2014, 10:52:04

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

pg_dump_v2.patch

Re: pg_dump reporing version of server & pg_dump as comments in the output

From

Abhijit Menon-Sen

Date:

01 July 2014, 22:37:10

At 2014-06-17 13:21:34 +0530, jeevan.chalke@enterprisedb.com wrote:
>
> Anyone has any other views ?

I guess nobody has strong feelings either way. I've marked this
(i.e. your slightly-revised patch) ready for committer.

-- Abhijit

Re: pg_dump reporing version of server & pg_dump as comments in the output

From

Tom Lane

Date:

08 July 2014, 02:07:16

Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
> 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)

I committed this with slight further rearrangement to create what seems
like better vertical whitespace choices in verbose vs non-verbose cases,
to wit

--
-- PostgreSQL database dump
--

-- Dumped from database version 9.5devel
-- Dumped by pg_dump version 9.5devel

SET statement_timeout = 0;
...

vs

--
-- PostgreSQL database dump
--

-- Dumped from database version 9.5devel
-- Dumped by pg_dump version 9.5devel

-- Started on 2014-07-07 19:05:54 EDT

SET statement_timeout = 0;
...
        regards, tom lane