Skip site navigation (1) Skip section navigation (2)

REVIEW: alter extension upgrade (patch v3)

From: Anssi Kääriäinen <anssi(dot)kaariainen(at)thl(dot)fi>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>
Subject: REVIEW: alter extension upgrade (patch v3)
Date: 2011-02-02 15:08:55
Message-ID: (view raw, whole thread or download thread mbox)
Lists: pgsql-hackers
This review is mostly about trying to break stuff by using brute force. 
I am not skilled enough in C to do a code level review.

The patch applies cleanly after applying the extensions support for 
pg_dump patch. The patch is in context format.

There is one make check error:
test rules                    ... FAILED

The regression.diffs file is attached.

When building the contrib modules, make installcheck has one failure:
test dblink                   ... FAILED

The regression.diffs for this is attached as contrib_regression.diffs

I don't know too well how the regression tests are supposed to be run, 
so error on my part is very much possible.

There is no upgrade rule for adminpack. If I am not mistaken, this could 
be defined as
     upgrade_from_all = '.* => adminpack.sql'
as the adminpack.sql only contains create or replace statements.

dblink has upgrade rule
     upgrade_from_null = 'null => dblink.upgrade.sql'
however, there is not file dblink.upgrade.sql. Same for btree_gin and 
possibly others.

For example the int_aggregate.upgrade.sql uses this form:
     alter function @extschema(at)(dot)int_array_enum(integer[]) set extension 
while btree_gist has this (no @extschema@):
     alter function gbt_text_compress(internal) set extension btree_gist;
I am not sure which one of them is correct. I think having extschema 
there is what is wanted, and if so, this should also be mentioned in the 

In general, I think the upgrade rules and associated .upgrade.sql files 
should be revisited. I did not have time to go through them all.

Might it be possible that there is a risk of attaching incompatible 
versions of objects to an extension? Say, you have contrib module foo 
which is loaded in 8.4 with \i. The module contains function bar, which 
is redefined in 9.1. When you upgrade from null, you will have the 8.4's 
version of function bar registered in the extension foo. That is, 
upgrade from null does not upgrade the objects in an extension, it just 
attaches them to the extension. As a result, when restoring from dump, 
you get back 9.1's version.

What will happen if you have tables, indexes etc depending on the 
objects in the extension? Is it possible to create rules to upgrade 
these extensions easily?

As the operator used for matching the version number is ~, the rule ' => 
foo.upgrade.sql' will match any version. This is just as specified, but 
if you have two upgrade rules: '9.1 => foo.upgrade.sql.1' and '9.1.1 => 
foo.upgrade.sql.2' the regexp for 9.1 will match also 9.1.1 . This could 
be quite surprising. Should the operator be such that exact match is 
required, that is ^ is inserted before the search string and $ after it? 
Also, should the ALTER EXTENSION UPGRADE give a notice: 'using rule 

While trying to fool the upgrade engine, I noticed that if you use 
absolute paths, you get the error:
    ERROR:  script path not allowed
    DETAIL:  Extension's script are expected in "/usr/local/pgsql/share".
Is this correct? Isn't the path "/usr/local/pgsql/share/contrib"

Is it intentional that paths are allowed in the upgrade script name? You 
can use ../../foobar.upgrade.sql as the filename.

All in all, so far the feature has been relatively easy and intuitive to 
use. My biggest concern is the upgrade_from_null, as specified, does not 
actually upgrade the objects, just attach them to the extension.

I have no more time to test the patch. I hope this is helpful as is.

  - Anssi

Attachment: regression.diffs
Description: text/plain (16.8 KB)
Attachment: contrib_regression.diffs
Description: text/plain (37.5 KB)


pgsql-hackers by date

Next:From: Marko TiikkajaDate: 2011-02-02 15:24:40
Subject: Re: Transaction-scope advisory locks
Previous:From: Dimitri FontaineDate: 2011-02-02 14:45:45

Privacy Policy | About PostgreSQL
Copyright © 1996-2018 The PostgreSQL Global Development Group