Thread: pg_upgrade and extra_float_digits

pg_upgrade and extra_float_digits

From
Bruce Momjian
Date:
FYI, I test pg_upgrade by loading the old cluster's regression database
from a pg_dump output file, then after the upgrade, I dump the
regression database of the new cluster and diff the changes.

The problem I just encountered is that pg_dump uses
extra_float_digits=-3 for 9.0, while previous releases used '2'.  I had
to do hack each server version to get a dump output that would match
without rounding errors --- it did eventually work and validated.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com


Re: pg_upgrade and extra_float_digits

From
Andrew Dunstan
Date:

Bruce Momjian wrote:
> FYI, I test pg_upgrade by loading the old cluster's regression database
> from a pg_dump output file, then after the upgrade, I dump the
> regression database of the new cluster and diff the changes.
>
> The problem I just encountered is that pg_dump uses
> extra_float_digits=-3 for 9.0, while previous releases used '2'.  I had
> to do hack each server version to get a dump output that would match
> without rounding errors --- it did eventually work and validated.
>
>   

That sounds like a disaster waiting to happen. The server version is 
going to affect much more than just this behaviour, surely. Wouldn't it 
be better to provide a pg_dump option to provide the extra_float_digits 
setting?

cheers

andrew


Re: pg_upgrade and extra_float_digits

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
> 
> 
> Bruce Momjian wrote:
> > FYI, I test pg_upgrade by loading the old cluster's regression database
> > from a pg_dump output file, then after the upgrade, I dump the
> > regression database of the new cluster and diff the changes.
> >
> > The problem I just encountered is that pg_dump uses
> > extra_float_digits=-3 for 9.0, while previous releases used '2'.  I had
> > to do hack each server version to get a dump output that would match
> > without rounding errors --- it did eventually work and validated.
> >
> >   
> 
> That sounds like a disaster waiting to happen. The server version is 
> going to affect much more than just this behaviour, surely. Wouldn't it 
> be better to provide a pg_dump option to provide the extra_float_digits 
> setting?

FYI, you can't override it with PGOPTIONS because it is set inside the
pg_dump binary.  I am not sure what you mean by your second sentence.

I was just reporting it in case anyone else was trying this for testing.
I doubt anyone else is going to try such a thing.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com


Re: pg_upgrade and extra_float_digits

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> Bruce Momjian wrote:
>> FYI, I test pg_upgrade by loading the old cluster's regression database
>> from a pg_dump output file, then after the upgrade, I dump the
>> regression database of the new cluster and diff the changes.
>> 
>> The problem I just encountered is that pg_dump uses
>> extra_float_digits=-3 for 9.0, while previous releases used '2'.  I had
>> to do hack each server version to get a dump output that would match
>> without rounding errors --- it did eventually work and validated.

> That sounds like a disaster waiting to happen. The server version is 
> going to affect much more than just this behaviour, surely. Wouldn't it 
> be better to provide a pg_dump option to provide the extra_float_digits 
> setting?

What disaster?  That's only for test purposes, it has nothing to do with
actual data transfer.
        regards, tom lane


Re: pg_upgrade and extra_float_digits

From
Andrew Dunstan
Date:

Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>   
>> Bruce Momjian wrote:
>>     
>>> FYI, I test pg_upgrade by loading the old cluster's regression database
>>> from a pg_dump output file, then after the upgrade, I dump the
>>> regression database of the new cluster and diff the changes.
>>>
>>> The problem I just encountered is that pg_dump uses
>>> extra_float_digits=-3 for 9.0, while previous releases used '2'.  I had
>>> to do hack each server version to get a dump output that would match
>>> without rounding errors --- it did eventually work and validated.
>>>       
>
>   
>> That sounds like a disaster waiting to happen. The server version is 
>> going to affect much more than just this behaviour, surely. Wouldn't it 
>> be better to provide a pg_dump option to provide the extra_float_digits 
>> setting?
>>     
>
> What disaster?  That's only for test purposes, it has nothing to do with
> actual data transfer.
>
>             
>   

Maybe I have misunderstood. How exactly is the server version being 
hacked here? I know it's only for testing, but it still seems to me that 
lying to a program as heavily version dependant as pg_dump is in general 
a bad idea.

cheers

andrew




Re: pg_upgrade and extra_float_digits

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
> >>> The problem I just encountered is that pg_dump uses
> >>> extra_float_digits=-3 for 9.0, while previous releases used '2'.  I had
> >>> to do hack each server version to get a dump output that would match
> >>> without rounding errors --- it did eventually work and validated.
> >>>       
> >
> >   
> >> That sounds like a disaster waiting to happen. The server version is 
> >> going to affect much more than just this behaviour, surely. Wouldn't it 
> >> be better to provide a pg_dump option to provide the extra_float_digits 
> >> setting?
> >>     
> >
> > What disaster?  That's only for test purposes, it has nothing to do with
> > actual data transfer.
> >
> >             
> >   
> 
> Maybe I have misunderstood. How exactly is the server version being 
> hacked here? I know it's only for testing, but it still seems to me that 
> lying to a program as heavily version dependant as pg_dump is in general 
> a bad idea.

The code in pg_dump 9.0 is:
   /*    * If supported, set extra_float_digits so that we can dump float data    * exactly (given correctly
implementedfloat I/O code, anyway)    */   if (g_fout->remoteVersion >= 90000)       do_sql_command(g_conn, "SET
extra_float_digitsTO 3");   else if (g_fout->remoteVersion >= 70400)
 
-->     do_sql_command(g_conn, "SET extra_float_digits TO 2");

The indicated line had to be changed to '3'.  I did not change anything
else, and it was only done in my private CVS tree.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com


Re: pg_upgrade and extra_float_digits

From
Andrew Dunstan
Date:

Bruce Momjian wrote:
>>
>> Maybe I have misunderstood. How exactly is the server version being 
>> hacked here? I know it's only for testing, but it still seems to me that 
>> lying to a program as heavily version dependant as pg_dump is in general 
>> a bad idea.
>>     
>
> The code in pg_dump 9.0 is:
>
>     /*
>      * If supported, set extra_float_digits so that we can dump float data
>      * exactly (given correctly implemented float I/O code, anyway)
>      */
>     if (g_fout->remoteVersion >= 90000)
>         do_sql_command(g_conn, "SET extra_float_digits TO 3");
>     else if (g_fout->remoteVersion >= 70400)
> -->     do_sql_command(g_conn, "SET extra_float_digits TO 2");
>
> The indicated line had to be changed to '3'.  I did not change anything
> else, and it was only done in my private CVS tree.
>
>   

Oh, I see. It is pg_dump that you hacked. That wasn't clear to me from 
what you first said.

But do earlier server versions accept a value of 3? The 8.4 docs say 
"The value can be set as high as 2".

cheers

andrew


Re: pg_upgrade and extra_float_digits

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
> 
> 
> Bruce Momjian wrote:
> >>
> >> Maybe I have misunderstood. How exactly is the server version being 
> >> hacked here? I know it's only for testing, but it still seems to me that 
> >> lying to a program as heavily version dependant as pg_dump is in general 
> >> a bad idea.
> >>     
> >
> > The code in pg_dump 9.0 is:
> >
> >     /*
> >      * If supported, set extra_float_digits so that we can dump float data
> >      * exactly (given correctly implemented float I/O code, anyway)
> >      */
> >     if (g_fout->remoteVersion >= 90000)
> >         do_sql_command(g_conn, "SET extra_float_digits TO 3");
> >     else if (g_fout->remoteVersion >= 70400)
> > -->     do_sql_command(g_conn, "SET extra_float_digits TO 2");
> >
> > The indicated line had to be changed to '3'.  I did not change anything
> > else, and it was only done in my private CVS tree.
> >
> >   
> 
> Oh, I see. It is pg_dump that you hacked. That wasn't clear to me from 
> what you first said.
> 
> But do earlier server versions accept a value of 3? The 8.4 docs say 
> "The value can be set as high as 2".

That is the other thing I had to hack --- the 8.4 backend version had to
be changed to accept '3'.  The good thing is this has to be done only
once --- once I have the dump file, I can use it in testing repeatedly
because 8.4 does not change.

Eventually the idea would be to have the build farm run such tests (with
a properly created dump file) so we can learn quickly if the backend
data format is changed.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com


Re: pg_upgrade and extra_float_digits

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> Andrew Dunstan wrote:
>> But do earlier server versions accept a value of 3? The 8.4 docs say 
>> "The value can be set as high as 2".

> That is the other thing I had to hack --- the 8.4 backend version had to
> be changed to accept '3'.  The good thing is this has to be done only
> once --- once I have the dump file, I can use it in testing repeatedly
> because 8.4 does not change.

> Eventually the idea would be to have the build farm run such tests (with
> a properly created dump file) so we can learn quickly if the backend
> data format is changed.

If we're thinking of doing that, it would be better to back-patch the
change that allowed '3'.
        regards, tom lane


Re: pg_upgrade and extra_float_digits

From
Andrew Dunstan
Date:

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
>   
>> Andrew Dunstan wrote:
>>     
>>> But do earlier server versions accept a value of 3? The 8.4 docs say 
>>> "The value can be set as high as 2".
>>>       
>
>   
>> That is the other thing I had to hack --- the 8.4 backend version had to
>> be changed to accept '3'.  The good thing is this has to be done only
>> once --- once I have the dump file, I can use it in testing repeatedly
>> because 8.4 does not change.
>>     
>
>   
>> Eventually the idea would be to have the build farm run such tests (with
>> a properly created dump file) so we can learn quickly if the backend
>> data format is changed.
>>     
>
> If we're thinking of doing that, it would be better to back-patch the
> change that allowed '3'.
>
>             
>   

Yeah.

It's going to require some fancy dancing to get the buildfarm to do it. 
Each buildfarm run is for a specific branch, and all the built artefacts 
are normally thrown away. I'd have to work out a way of stashing the 
binaries from a build on one branch for use in the pg_upgrade tests in 
the run on another branch. It's doable but could get messy.


cheers

andrew


Re: pg_upgrade and extra_float_digits

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
> >> Eventually the idea would be to have the build farm run such tests (with
> >> a properly created dump file) so we can learn quickly if the backend
> >> data format is changed.
> >>     
> >
> > If we're thinking of doing that, it would be better to back-patch the
> > change that allowed '3'.
> >
> >             
> >   
> 
> Yeah.
> 
> It's going to require some fancy dancing to get the buildfarm to do it. 
> Each buildfarm run is for a specific branch, and all the built artefacts 
> are normally thrown away. I'd have to work out a way of stashing the 
> binaries from a build on one branch for use in the pg_upgrade tests in 
> the run on another branch. It's doable but could get messy.

Uh, that is not actually a problem.  You just need to set
extra_float_digits=-3 to create the dump file, which is only done once
for each major version.  You can _load_ that dump file into an
unmodified old cluster and test just fine.  I will write up some
instructions in the next few days.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com


Re: pg_upgrade and extra_float_digits

From
Andrew Dunstan
Date:

Bruce Momjian wrote:
> Andrew Dunstan wrote:
>   
>>>> Eventually the idea would be to have the build farm run such tests (with
>>>> a properly created dump file) so we can learn quickly if the backend
>>>> data format is changed.
>>>>     
>>>>         
>>> If we're thinking of doing that, it would be better to back-patch the
>>> change that allowed '3'.
>>>
>>>             
>>>   
>>>       
>> Yeah.
>>
>> It's going to require some fancy dancing to get the buildfarm to do it. 
>> Each buildfarm run is for a specific branch, and all the built artefacts 
>> are normally thrown away. I'd have to work out a way of stashing the 
>> binaries from a build on one branch for use in the pg_upgrade tests in 
>> the run on another branch. It's doable but could get messy.
>>     
>
> Uh, that is not actually a problem.  You just need to set
> extra_float_digits=-3 to create the dump file, which is only done once
> for each major version.  You can _load_ that dump file into an
> unmodified old cluster and test just fine.  I will write up some
> instructions in the next few days.
>
>   

You are missing the point I was making. A buildfarm run does not 
normally have available to it any binaries for a version other that the 
one it is building. There is no notion of a multi-branch buildfarm run. 
Each run is for a particular branch and is a separate miracle.  So I'm 
not concerned about the structure of the dump file but about what will 
be used to load it into an old cluster during a buildfarm run.

cheers

andrew


Re: pg_upgrade and extra_float_digits

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
> > Uh, that is not actually a problem.  You just need to set
> > extra_float_digits=-3 to create the dump file, which is only done once
> > for each major version.  You can _load_ that dump file into an
> > unmodified old cluster and test just fine.  I will write up some
> > instructions in the next few days.
> >
> >   
> 
> You are missing the point I was making. A buildfarm run does not 
> normally have available to it any binaries for a version other that the 
> one it is building. There is no notion of a multi-branch buildfarm run. 
> Each run is for a particular branch and is a separate miracle.  So I'm 
> not concerned about the structure of the dump file but about what will 
> be used to load it into an old cluster during a buildfarm run.

Thank you.  I understand now.

Imagine finding out on the build farm right away when we break binary
compatibility --- that would be cool.  However, that might be overkill. 
My testing seems to be working just fine.  In fact the only diff I see
is:
< CREATE PROCEDURAL LANGUAGE plpgsql;---> CREATE OR REPLACE PROCEDURAL LANGUAGE plpgsql;

and that is a known change.  I might end up adding my regression dump
files to our ftp site (400k for each major version), and just having
people use them for testing.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com


Re: pg_upgrade and extra_float_digits

From
Andrew Dunstan
Date:

Bruce Momjian wrote:
> Thank you.  I understand now.
>
> Imagine finding out on the build farm right away when we break binary
> compatibility --- that would be cool.  
>   

I'm not saying we can't do that, just that it will not be a trivial 
change. And yes it would be cool, although I hope we would know before 
we committed such a change that that would be the outcome.

cheers

andrew


Re: pg_upgrade and extra_float_digits

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> Andrew Dunstan wrote:
>> It's going to require some fancy dancing to get the buildfarm to do it. 
>> Each buildfarm run is for a specific branch, and all the built artefacts 
>> are normally thrown away.

> Uh, that is not actually a problem.  You just need to set
> extra_float_digits=-3 to create the dump file, which is only done once
> for each major version.

Wrong.  In the first place, we're not going to start carrying something
as large as a pg_dump of the regression database as part of the source
code for the buildfarm.  Even if we wanted to, it wouldn't work because
the results aren't platform-independent --- there are float differences
and probably row ordering differences to worry about.  In the second
place, it won't "only be done once", unless you imagine that we never
change the regression tests for back branches; a casual perusal of the
CVS logs will disprove that idea.

The only thing that's really going to work here is to generate the dump
on the fly.
        regards, tom lane


Re: pg_upgrade and extra_float_digits

From
Andrew Dunstan
Date:

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
>   
>> Andrew Dunstan wrote:
>>     
>>> It's going to require some fancy dancing to get the buildfarm to do it. 
>>> Each buildfarm run is for a specific branch, and all the built artefacts 
>>> are normally thrown away.
>>>       
>
>   
>> Uh, that is not actually a problem.  You just need to set
>> extra_float_digits=-3 to create the dump file, which is only done once
>> for each major version.
>>     
>
> Wrong.  In the first place, we're not going to start carrying something
> as large as a pg_dump of the regression database as part of the source
> code for the buildfarm.  Even if we wanted to, it wouldn't work because
> the results aren't platform-independent --- there are float differences
> and probably row ordering differences to worry about.  In the second
> place, it won't "only be done once", unless you imagine that we never
> change the regression tests for back branches; a casual perusal of the
> CVS logs will disprove that idea.
>
> The only thing that's really going to work here is to generate the dump
> on the fly.
>
>             
>   

This whole discussion leads me to the conclusion that we need to look 
more imaginatively at our testing regime. When the buildfarm was created 
it (via pg_regress) covered a lot of what we needed to test, but that is 
becoming less and less true. Not only does pg_upgrade need testing but 
we need to devise some sort of automated testing regime for SR and HS, 
among other things. pg_regress is showing it's age a bit, I think.

cheers

andrew


Re: pg_upgrade and extra_float_digits

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> This whole discussion leads me to the conclusion that we need to look 
> more imaginatively at our testing regime. When the buildfarm was created 
> it (via pg_regress) covered a lot of what we needed to test, but that is 
> becoming less and less true. Not only does pg_upgrade need testing but 
> we need to devise some sort of automated testing regime for SR and HS, 
> among other things. pg_regress is showing it's age a bit, I think.

The regression tests have never pretended to test more than a fraction
of what might be interesting to test.  Crash recovery, in particular,
has always been interesting and has never been tested in any mechanized
way.  They don't really exercise concurrent behavior in any meaningful
way either.  I don't think they're "showing their age" so much as we're
starting to get more ambitious about what we would like to have routine
testing for.
        regards, tom lane


Re: pg_upgrade and extra_float_digits

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Andrew Dunstan wrote:
> >> It's going to require some fancy dancing to get the buildfarm to do it. 
> >> Each buildfarm run is for a specific branch, and all the built artefacts 
> >> are normally thrown away.
> 
> > Uh, that is not actually a problem.  You just need to set
> > extra_float_digits=-3 to create the dump file, which is only done once
> > for each major version.
> 
> Wrong.  In the first place, we're not going to start carrying something
> as large as a pg_dump of the regression database as part of the source
> code for the buildfarm.  Even if we wanted to, it wouldn't work because
> the results aren't platform-independent --- there are float differences
> and probably row ordering differences to worry about.  In the second

Oh, yea.

> place, it won't "only be done once", unless you imagine that we never
> change the regression tests for back branches; a casual perusal of the
> CVS logs will disprove that idea.

Well, it doesn't have to match the regression test output exactly --- it
just has to be a valid sample.  I never run the regression tests as part
of my testing --- I only load my fixed pg_dump output into the old
database and dump them from the new, and diff.

> The only thing that's really going to work here is to generate the dump
> on the fly.

Well, to do it on the fly, you need to:
use $libdir for regression .so files, not absolute pathschange CREATE OR REPLACE LANGUAGE to simple CREAtE for 8.4run
ittwice to fix inheritance COPY column orderingdeal with extra_float_digits
 

That sounds tricky.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com


Re: pg_upgrade and extra_float_digits

From
Bruce Momjian
Date:
Bruce Momjian wrote:
> Well, to do it on the fly, you need to:
>
>     use $libdir for regression .so files, not absolute paths
>     change CREATE OR REPLACE LANGUAGE to simple CREAtE for 8.4
>     run it twice to fix inheritance COPY column ordering
>     deal with extra_float_digits
>
> That sounds tricky.

I have added the attached file to CVS to explain the proper pg_upgrade
testing method.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com
The most effective way to test pg_upgrade, aside from testing on user
data, is by upgrading the PostgreSQL regression database.

This testing process first requires the creation of a valid regression
database dump.  Such files contain most database features and are
specific to each major version of Postgres.

Here are the steps needed to create a regression database dump file:

1)  Create and populate the regression database in the old cluster
    This database can be created by running 'gmake installcheck' from
    src/test/regression.

2)  Use pg_dump to dump out the regression database
    Use the new cluster's pg_dump on the old database to minimize
    whitespace differences in the diff.

3)  Adjust the regression database dump file

    a)  Change CREATE FUNCTION shared object paths to use '$libdir'
        The old and new cluster will have different shared object paths.

    b)  Remove 'regex_flavor' (not supported in Postgres 9.0)

    c)  Change CREATE OR REPLACE LANGUAGE to CREATE LANGUAGE
        The former syntax is only supported in Postgres 9.0.

    d)  Perform the load/dump twice
        This fixes problems with the ordering of COPY columns for
        inherited tables.

    e)  Fix any wrapping format differences
        Commands like CREATE TRIGGER and ALTER TABLE sometimes have
        differences.

    f)  Adjust extra_float_digits
    Postgres 9.0 pg_dump uses extra_float_digits=-2 for pre-9.0
    databases, and extra_float_digits=-3 for >= 9.0 databases.
    It is necessary to modify 9.0 pg_dump to always use -3, and
    modify the pre-9.0 old server to accept extra_float_digits=-3.

Once the dump is created, it can be repeatedly loaded into the old
database, upgraded, and dumped out of the new database, and then
compared to the original version. To test the dump file, perform these
steps:

1)  Create the old and new clusters in different directories.

2)  Copy the regression shared object files into the appropriate /lib
    directory for old and new clusters.

3)  Create the regression database in the old server.

4)  Load the dump file created above into the regression database;
    check for errors while loading.

5)  Upgrade the old database to the new major version, as outlined in
    the pg_upgrade manual section.

6)  Use pg_dump to dump out the regression database in the new cluster.

7)  Diff the regression database dump file with the regression dump
    file loaded into the old server.