Re: Hardening pg_upgrade

Now that everyone is happy with how pg_upgrade_support uses global
variables to set preserved oids (or at least has no better ideas), I
think it is time to lock down this usage to prevent future breakage.

Specifically, the first attached patch causes pg_upgrade_support
functions to throw errors when called by a backend that is not in binary
upgrade mode. (This seems like a good safety measure.) Second, and
more importantly, the patch prevents automatic oid assignment when in
binary upgrade mode, except for temporary objects. This is to help
guarantee that system-assigned oids do not conflict with preserved oids.

I had to make an exception for temporary tables because pg_upgrade uses
temporary tables to collect schema information. I tried writing the
query to use CTEs (second patch), but I would then have to have one
query for 8.3, which doesn't support CTEs, and another for 8.4+, plus
the CTE query was more complex than I liked. Another idea would be to
drop 8.3 support (and remove lots of code to support that), but the
recent large increase in the number of people upgrading from 8.4 makes
that unattractive. (8.3 did use a different timestamp storage format
though.)

(Of course, the assumption is that temporary tables will not exist at
the time you are assigning preserved oids.)

Barring objections, I plan to apply the first patch to head, and discard
the second patch.

FYI, I think the macro isTempOrToastNamespace() is misnamed --- it is
testing for temporary tables or temporary toast tables, not for any
toast table. Seems it should be called isTempOrToastTempNamespace().
Should I rename it in a separate commit?

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

+ Everyone has their own god. +

Bruce Momjian wrote
> I had to make an exception for temporary tables because pg_upgrade uses
> temporary tables to collect schema information. I tried writing the
> query to use CTEs (second patch), but I would then have to have one
> query for 8.3, which doesn't support CTEs, and another for 8.4+, plus
> the CTE query was more complex than I liked. Another idea would be to
> drop 8.3 support (and remove lots of code to support that), but the
> recent large increase in the number of people upgrading from 8.4 makes
> that unattractive. (8.3 did use a different timestamp storage format
> though.)

Why not tell people on 8.3- that a direct upgrade is not supported but that
an indirect upgrade to 9.4 or earlier has to be performed first and then
that can be upgraded to 9.5+ ?

I'm not clear on how the 8.4 upgrades volume impacts a decision to support
8.3- upgrades?

David J.

--
View this message in context: http://postgresql.1045698.n5.nabble.com/Hardening-pg-upgrade-tp5815735p5815748.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

On Thu, Aug 21, 2014 at 11:43:42AM -0700, David G Johnston wrote:
> Bruce Momjian wrote
> > I had to make an exception for temporary tables because pg_upgrade uses
> > temporary tables to collect schema information. I tried writing the
> > query to use CTEs (second patch), but I would then have to have one
> > query for 8.3, which doesn't support CTEs, and another for 8.4+, plus
> > the CTE query was more complex than I liked. Another idea would be to
> > drop 8.3 support (and remove lots of code to support that), but the
> > recent large increase in the number of people upgrading from 8.4 makes
> > that unattractive. (8.3 did use a different timestamp storage format
> > though.)
>
> Why not tell people on 8.3- that a direct upgrade is not supported but that
> an indirect upgrade to 9.4 or earlier has to be performed first and then
> that can be upgraded to 9.5+ ?

Yes, we could easily do that, and trim down pg_upgrade in the process.
Are people OK with that?

> I'm not clear on how the 8.4 upgrades volume impacts a decision to support
> 8.3- upgrades?

My point is that people aren't doing upgrades just from 9.1 and 9.2, but
often from very old releases. and the end-of-lifed of 8.4 prompted a lot
of people to upgrade. Now, since 8.3 has been end-of-lifed since
February, 2013, we might be able to argue that 8.3 already had a year to
upgrade, so if they now want to upgrade, they have to do it in two
steps.

Anyway, I think we need more opinions on this.

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

+ Everyone has their own god. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Specifically, the first attached patch causes pg_upgrade_support
> functions to throw errors when called by a backend that is not in binary
> upgrade mode. (This seems like a good safety measure.)

Agreed about that part.

> Second, and
> more importantly, the patch prevents automatic oid assignment when in
> binary upgrade mode, except for temporary objects. This is to help
> guarantee that system-assigned oids do not conflict with preserved oids.

> I had to make an exception for temporary tables because pg_upgrade uses
> temporary tables to collect schema information.

This seems like a bad idea. If you are going to have such an off-switch
at all (which I'm not sure I buy the need for), it should not have holes
in it.

> I tried writing the
> query to use CTEs (second patch), but I would then have to have one
> query for 8.3, which doesn't support CTEs, and another for 8.4+, plus
> the CTE query was more complex than I liked. Another idea would be to
> drop 8.3 support (and remove lots of code to support that), but the
> recent large increase in the number of people upgrading from 8.4 makes
> that unattractive. (8.3 did use a different timestamp storage format
> though.)

I vote for discarding 8.3 support in pg_upgrade. There are already enough
limitations on pg_upgrade from pre-8.4 to make it of questionable value;
if it's going to create problems like this, it's time to cut the rope.

regards, tom lane

On Thu, Aug 21, 2014 at 10:07 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> I tried writing the
>> query to use CTEs (second patch), but I would then have to have one
>> query for 8.3, which doesn't support CTEs, and another for 8.4+, plus
>> the CTE query was more complex than I liked. Another idea would be to
>> drop 8.3 support (and remove lots of code to support that), but the
>> recent large increase in the number of people upgrading from 8.4 makes
>> that unattractive. (8.3 did use a different timestamp storage format
>> though.)
>
> I vote for discarding 8.3 support in pg_upgrade. There are already enough
> limitations on pg_upgrade from pre-8.4 to make it of questionable value;
> if it's going to create problems like this, it's time to cut the rope.

+1. 8.3 has been unsupported for a fairly long time now, and you can
still do a two-step upgrade if you're on that old a version.

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

--On 21. August 2014 22:08:58 +0200 Magnus Hagander <magnus(at)hagander(dot)net>
wrote:

>> I vote for discarding 8.3 support in pg_upgrade. There are already
>> enough limitations on pg_upgrade from pre-8.4 to make it of questionable
>> value; if it's going to create problems like this, it's time to cut the
>> rope.
>
> +1. 8.3 has been unsupported for a fairly long time now, and you can
> still do a two-step upgrade if you're on that old a version.

Also +1 from my side. I've seen some old 8.3 installations at customers,
still, but they aren't large and can easily be upgraded with a two step
upgrade.

--
Thanks

Bernd

Bernd Helmle <mailings(at)oopsware(dot)de> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> wrote:

>>> I vote for discarding 8.3 support in pg_upgrade.  There are already
>>> enough limitations on pg_upgrade from pre-8.4 to make it of questionable
>>> value; if it's going to create problems like this, it's time to cut the
>>> rope.
>>
>> +1. 8.3 has been unsupported for a fairly long time now, and you can
>> still do a two-step upgrade if you're on that old a version.
>
> Also +1 from my side. I've seen some old 8.3 installations at customers,
> still, but they aren't large and can easily be upgraded with a two step
> upgrade.

+1

If we could leave it without it being any extra work, fine; but
once a release is over a year out of support, if it's a matter of
putting extra work on the pg hackers or on the users who have
chosen to wait more than a year after support ends to do the
upgrade, I'm OK with asking those users to do a two-phase upgrade
or fall back to pg_dump.  It's not like we're leaving them without
any options.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, Aug 25, 2014 at 06:34:12AM -0700, Kevin Grittner wrote:
> Bernd Helmle <mailings(at)oopsware(dot)de> wrote:
> > Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>
> >>> I vote for discarding 8.3 support in pg_upgrade.  There are already
> >>> enough limitations on pg_upgrade from pre-8.4 to make it of questionable
> >>> value; if it's going to create problems like this, it's time to cut the
> >>> rope.
> >>
> >> +1. 8.3 has been unsupported for a fairly long time now, and you can
> >> still do a two-step upgrade if you're on that old a version.
> >
> > Also +1 from my side. I've seen some old 8.3 installations at customers,
> > still, but they aren't large and can easily be upgraded with a two step
> > upgrade.
>
> +1
>
> If we could leave it without it being any extra work, fine; but
> once a release is over a year out of support, if it's a matter of
> putting extra work on the pg hackers or on the users who have
> chosen to wait more than a year after support ends to do the
> upgrade, I'm OK with asking those users to do a two-phase upgrade
> or fall back to pg_dump.  It's not like we're leaving them without
> any options.

OK, I will move in the direction of removing 8.3 support and use a
single query to pull schema information. I was hesistant to remove 8.3
support as I know we have kept pg_dump support all the way back to 7.0,
but it seems pg_upgrade need not have the same version requirements.

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

+ Everyone has their own god. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> OK, I will move in the direction of removing 8.3 support and use a
> single query to pull schema information. I was hesistant to remove 8.3
> support as I know we have kept pg_dump support all the way back to 7.0,
> but it seems pg_upgrade need not have the same version requirements.

Not really related, but ... I've been thinking that it's time to rip out
pg_dump's support for server versions before 7.3 or 7.4. That would let
us get rid of a lot of klugy code associated with the lack of schemas
and dependency info in the older versions. It's possible that we should
move the cutoff even further --- I've not looked closely at how much could
be removed by dropping versions later than 7.3.

Aside from the question of how much old code could be removed, there's the
salient point of how do we test pg_dump against such old branches? The
further back you go the harder it is to even build PG on modern platforms,
and the less likely it will work (I note for example that pre-8.0
configure doesn't try to use -fwrapv, let alone some of the other switches
we've found necessary on recent gcc). I've usually tested pg_dump patches
against old servers by running them against builds I have in captivity on
my old HPPA box ... but once that dies, I'm *not* looking forward to
trying to rebuild 7.x on my current machines.

regards, tom lane

On Mon, Aug 25, 2014 at 03:04:52PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > OK, I will move in the direction of removing 8.3 support and use a
> > single query to pull schema information. I was hesistant to remove 8.3
> > support as I know we have kept pg_dump support all the way back to 7.0,
> > but it seems pg_upgrade need not have the same version requirements.
>
> Not really related, but ... I've been thinking that it's time to rip out
> pg_dump's support for server versions before 7.3 or 7.4. That would let
> us get rid of a lot of klugy code associated with the lack of schemas
> and dependency info in the older versions. It's possible that we should
> move the cutoff even further --- I've not looked closely at how much could
> be removed by dropping versions later than 7.3.

Yeah, it kind of is related, as that was the logic I followed originally
for pg_upgrade, i.e. never remove supported versions --- that has been
overridden.

> Aside from the question of how much old code could be removed, there's the
> salient point of how do we test pg_dump against such old branches? The
> further back you go the harder it is to even build PG on modern platforms,
> and the less likely it will work (I note for example that pre-8.0
> configure doesn't try to use -fwrapv, let alone some of the other switches
> we've found necessary on recent gcc). I've usually tested pg_dump patches
> against old servers by running them against builds I have in captivity on
> my old HPPA box ... but once that dies, I'm *not* looking forward to
> trying to rebuild 7.x on my current machines.

Yes. You could argue that a double-upgrade from 7.0 to 8.0 to 9.4 would
be less buggy than one from 7.0 to 9.4. I agree there is almost zero
testing of very old versions.

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

+ Everyone has their own god. +

On Mon, Aug 25, 2014 at 02:15:18PM -0400, Bruce Momjian wrote:
> On Mon, Aug 25, 2014 at 06:34:12AM -0700, Kevin Grittner wrote:
> > Bernd Helmle <mailings(at)oopsware(dot)de> wrote:
> > > Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> >
> > >>> I vote for discarding 8.3 support in pg_upgrade.  There are already
> > >>> enough limitations on pg_upgrade from pre-8.4 to make it of questionable
> > >>> value; if it's going to create problems like this, it's time to cut the
> > >>> rope.
> > >>
> > >> +1. 8.3 has been unsupported for a fairly long time now, and you can
> > >> still do a two-step upgrade if you're on that old a version.
> > >
> > > Also +1 from my side. I've seen some old 8.3 installations at customers,
> > > still, but they aren't large and can easily be upgraded with a two step
> > > upgrade.
> >
> > +1
> >
> > If we could leave it without it being any extra work, fine; but
> > once a release is over a year out of support, if it's a matter of
> > putting extra work on the pg hackers or on the users who have
> > chosen to wait more than a year after support ends to do the
> > upgrade, I'm OK with asking those users to do a two-phase upgrade
> > or fall back to pg_dump.  It's not like we're leaving them without
> > any options.
>
> OK, I will move in the direction of removing 8.3 support and use a
> single query to pull schema information. I was hesistant to remove 8.3
> support as I know we have kept pg_dump support all the way back to 7.0,
> but it seems pg_upgrade need not have the same version requirements.

I will modify pg_upgrade in three steps:

o remove 8.3 support
o use a single CTE rather than use a temp table
o harden the backend to prevent automatic oid assignment

This is all for 9.5.

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

+ Everyone has their own god. +

On Mon, Aug 25, 2014 at 06:34:38PM -0400, Bruce Momjian wrote:
> > OK, I will move in the direction of removing 8.3 support and use a
> > single query to pull schema information. I was hesistant to remove 8.3
> > support as I know we have kept pg_dump support all the way back to 7.0,
> > but it seems pg_upgrade need not have the same version requirements.
>
> I will modify pg_upgrade in three steps:
>
> o remove 8.3 support
> o use a single CTE rather than use a temp table
> o harden the backend to prevent automatic oid assignment
>
> This is all for 9.5.

Done.

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

+ Everyone has their own god. +

On Mon, Aug 25, 2014 at 3:04 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> OK, I will move in the direction of removing 8.3 support and use a
>> single query to pull schema information. I was hesistant to remove 8.3
>> support as I know we have kept pg_dump support all the way back to 7.0,
>> but it seems pg_upgrade need not have the same version requirements.
>
> Not really related, but ... I've been thinking that it's time to rip out
> pg_dump's support for server versions before 7.3 or 7.4. That would let
> us get rid of a lot of klugy code associated with the lack of schemas
> and dependency info in the older versions. It's possible that we should
> move the cutoff even further --- I've not looked closely at how much could
> be removed by dropping versions later than 7.3.
>
> Aside from the question of how much old code could be removed, there's the
> salient point of how do we test pg_dump against such old branches? The
> further back you go the harder it is to even build PG on modern platforms,
> and the less likely it will work (I note for example that pre-8.0
> configure doesn't try to use -fwrapv, let alone some of the other switches
> we've found necessary on recent gcc). I've usually tested pg_dump patches
> against old servers by running them against builds I have in captivity on
> my old HPPA box ... but once that dies, I'm *not* looking forward to
> trying to rebuild 7.x on my current machines.

I think it's fine for us to start requiring two-step upgrades beyond a
certain point, and I think removing pg_dump support for pre-7.4
versions of the server is very reasonable. I also think removing
pg_upgrade support for 8.3 is reasonable. Many products require
multi-step upgrades when crossing multiple release versions, and I
think we can, too.

Having said that, there are obviously advantages for our users if we
don't get too crazy about requiring that. I've used products in the
past where to get from version 3 to version 11 you have to upgrade
from 3 to 5, then 5 to 7, then 7 to 9, and then 9 to 11. That's
somewhat understandable from the vendor's point of view, but it's not
a lot of fun, and I think we should definitely avoid imposing those
kinds of requirements on our users.

What we're talking about here is much milder than that. For the
pg_upgrade case, you can upgrade from 8.3 to any of 8.4, 9.0, 9.1,
9.2, 9.3, and 9.4. You only need to do a two-step upgrade if you want
to leapfrog more than 6 major release versions. That seems like a
wide-enough window that it shouldn't inconvenience many people. For
the pg_dump case, you can upgrade from 7.2 or 7.3 to 7.4, 8.0, 8.1,
8.2, 8.3, 8.4, 9.0, 9.1, 9.2, 9.3, or 9.4; that is, 11 or 12 major
releases. The number of people who want to skip more than a dozen
releases in a single upgrade should be very small, and we might
council those people that they'd be better off with a step-wise
upgrade for other reasons - like the application-level compatibility
breaks we've made over the years - anyway.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Wed, Aug 27, 2014 at 09:54:11AM -0400, Robert Haas wrote:
> Having said that, there are obviously advantages for our users if we
> don't get too crazy about requiring that. I've used products in the
> past where to get from version 3 to version 11 you have to upgrade
> from 3 to 5, then 5 to 7, then 7 to 9, and then 9 to 11. That's
> somewhat understandable from the vendor's point of view, but it's not
> a lot of fun, and I think we should definitely avoid imposing those
> kinds of requirements on our users.
>
> What we're talking about here is much milder than that. For the
> pg_upgrade case, you can upgrade from 8.3 to any of 8.4, 9.0, 9.1,
> 9.2, 9.3, and 9.4. You only need to do a two-step upgrade if you want
> to leapfrog more than 6 major release versions. That seems like a
> wide-enough window that it shouldn't inconvenience many people. For
> the pg_dump case, you can upgrade from 7.2 or 7.3 to 7.4, 8.0, 8.1,
> 8.2, 8.3, 8.4, 9.0, 9.1, 9.2, 9.3, or 9.4; that is, 11 or 12 major
> releases. The number of people who want to skip more than a dozen
> releases in a single upgrade should be very small, and we might
> council those people that they'd be better off with a step-wise
> upgrade for other reasons - like the application-level compatibility
> breaks we've made over the years - anyway.

Two things --- first, removing 8.3 support in pg_upgrade allowed me to
remove lots of dead code, so it was a win. Second, I think you need to
look at the time span from old to new versions to understand if a
double-step release is reasonable. If that 3-5-7-9 release step spans
two years, it is too short --- if it spans 15 years, it is probably fine
as few people would wait 15 years to upgrade.

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

+ Everyone has their own god. +

On Wed, Aug 27, 2014 at 10:13 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> On Wed, Aug 27, 2014 at 09:54:11AM -0400, Robert Haas wrote:
>> Having said that, there are obviously advantages for our users if we
>> don't get too crazy about requiring that. I've used products in the
>> past where to get from version 3 to version 11 you have to upgrade
>> from 3 to 5, then 5 to 7, then 7 to 9, and then 9 to 11. That's
>> somewhat understandable from the vendor's point of view, but it's not
>> a lot of fun, and I think we should definitely avoid imposing those
>> kinds of requirements on our users.
>>
>> What we're talking about here is much milder than that. For the
>> pg_upgrade case, you can upgrade from 8.3 to any of 8.4, 9.0, 9.1,
>> 9.2, 9.3, and 9.4. You only need to do a two-step upgrade if you want
>> to leapfrog more than 6 major release versions. That seems like a
>> wide-enough window that it shouldn't inconvenience many people. For
>> the pg_dump case, you can upgrade from 7.2 or 7.3 to 7.4, 8.0, 8.1,
>> 8.2, 8.3, 8.4, 9.0, 9.1, 9.2, 9.3, or 9.4; that is, 11 or 12 major
>> releases. The number of people who want to skip more than a dozen
>> releases in a single upgrade should be very small, and we might
>> council those people that they'd be better off with a step-wise
>> upgrade for other reasons - like the application-level compatibility
>> breaks we've made over the years - anyway.
>
> Two things --- first, removing 8.3 support in pg_upgrade allowed me to
> remove lots of dead code, so it was a win. Second, I think you need to
> look at the time span from old to new versions to understand if a
> double-step release is reasonable. If that 3-5-7-9 release step spans
> two years, it is too short --- if it spans 15 years, it is probably fine
> as few people would wait 15 years to upgrade.

Right, I agree with all of that and was not intending to dispute any of it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company