hstore extension version screwup

Started by Andrew Dunstanover 12 years ago10 messages
#1Andrew Dunstan
andrew@dunslane.net
1 attachment(s)

When adding json support to hstore, I made a major blunder and added the
new functionality to the existing sql script instead of bumping the
version, renaming the script and adding an update script.

This was lazy and there's no real excuse, although I will note that it
was a mistake far too easy to make. Perhaps as a warning indicator we
should remove write permissions from these files.

Anyway, I have had some discussions with Dimitri, and the best idea
seems to be that we should do all the above, but in the update script
use conditional logic that only adds the functions if they aren't
already there and dependent on the extension. In the release notes we
should advise anyone who has loaded hstore to run 'ALTER EXTENSION
hstore UPDATE TO '1.2';"

The minor downside of this is that the upgrade script will depend on
plpgsql be available. We'll need to note that too, although I don't
recall the last time I came across a site that didn't have it loaded.

See attached for details of what's proposed.

cheers

andrew

Attachments:

hstorefixtext/plain; charset=UTF-8; name=hstorefixDownload
diff --git a/contrib/hstore/hstore--1.1--1.2.sql b/contrib/hstore/hstore--1.1--1.2.sql
new file mode 100644
index 0000000..99b8a16
--- /dev/null
+++ b/contrib/hstore/hstore--1.1--1.2.sql
@@ -0,0 +1,47 @@
+/* contrib/hstore/hstore--1.1--1.2.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION hstore UPDATE TO '1.2'" to load this file. \quit
+
+
+-- A version of 1.1 was shipped with these objects mistakenly in 9.3.0.
+-- Therefore we only add them if we detect that they aren't already there and 
+-- dependent on the extension.
+
+DO LANGUAGE plpgsql
+
+$$
+
+BEGIN
+
+   PERFORM 1
+   FROM pg_proc p
+       JOIN  pg_depend d
+          ON p.proname = 'hstore_to_json_loose'
+            AND d.objid = p.oid
+            AND d.refclassid = 'pg_extension'::regclass
+       JOIN pg_extension x
+          ON d.refobjid = x.oid
+            AND  x.extname = 'hstore';
+
+   IF NOT FOUND
+   THEN 
+
+        CREATE FUNCTION hstore_to_json(hstore)
+        RETURNS json
+        AS 'MODULE_PATHNAME', 'hstore_to_json'
+        LANGUAGE C IMMUTABLE STRICT;
+
+        CREATE CAST (hstore AS json)
+          WITH FUNCTION hstore_to_json(hstore);
+
+        CREATE FUNCTION hstore_to_json_loose(hstore)
+        RETURNS json
+        AS 'MODULE_PATHNAME', 'hstore_to_json_loose'
+        LANGUAGE C IMMUTABLE STRICT;
+
+   END IF;
+
+END;
+
+$$;
diff --git a/contrib/hstore/hstore--1.1.sql b/contrib/hstore/hstore--1.2.sql
similarity index 100%
rename from contrib/hstore/hstore--1.1.sql
rename to contrib/hstore/hstore--1.2.sql
diff --git a/contrib/hstore/hstore.control b/contrib/hstore/hstore.control
index 4104e17..9daf5e2 100644
--- a/contrib/hstore/hstore.control
+++ b/contrib/hstore/hstore.control
@@ -1,5 +1,5 @@
 # hstore extension
 comment = 'data type for storing sets of (key, value) pairs'
-default_version = '1.1'
+default_version = '1.2'
 module_pathname = '$libdir/hstore'
 relocatable = true
#2Peter Eisentraut
peter_e@gmx.net
In reply to: Andrew Dunstan (#1)
Re: hstore extension version screwup

On Fri, 2013-09-27 at 13:23 -0400, Andrew Dunstan wrote:

This was lazy and there's no real excuse, although I will note that it
was a mistake far too easy to make. Perhaps as a warning indicator we
should remove write permissions from these files.

What do you mean by that?

See attached for details of what's proposed.

see broken builds on build farm

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Peter Eisentraut (#2)
Re: hstore extension version screwup

On 09/29/2013 09:58 PM, Peter Eisentraut wrote:

On Fri, 2013-09-27 at 13:23 -0400, Andrew Dunstan wrote:

This was lazy and there's no real excuse, although I will note that it
was a mistake far too easy to make. Perhaps as a warning indicator we
should remove write permissions from these files.

What do you mean by that?

Well if these are not meant to be changed then not being able to write
them in your git repo might be a clue to that.

Just a thought.

See attached for details of what's proposed.

see broken builds on build farm

Yeah, looks like I forgot about the make file. I did test this but with
a manual install. Will fix.

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Andrew Dunstan (#3)
Re: hstore extension version screwup

On Sun, 2013-09-29 at 22:33 -0400, Andrew Dunstan wrote:

Well if these are not meant to be changed then not being able to write
them in your git repo might be a clue to that.

Git doesn't support setting file permissions other than the executable
bit, so this is a nonstarter.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Andrew Dunstan
andrew@dunslane.net
In reply to: Peter Eisentraut (#4)
Re: hstore extension version screwup

On 09/29/2013 10:38 PM, Peter Eisentraut wrote:

On Sun, 2013-09-29 at 22:33 -0400, Andrew Dunstan wrote:

Well if these are not meant to be changed then not being able to write
them in your git repo might be a clue to that.

Git doesn't support setting file permissions other than the executable
bit, so this is a nonstarter.

Oh, didn't know that, I've certainly know other SCM systems that do.

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Jim Nasby
jim@nasby.net
In reply to: Andrew Dunstan (#5)
Re: hstore extension version screwup

On 9/29/13 9:41 PM, Andrew Dunstan wrote:

On 09/29/2013 10:38 PM, Peter Eisentraut wrote:

On Sun, 2013-09-29 at 22:33 -0400, Andrew Dunstan wrote:

Well if these are not meant to be changed then not being able to write
them in your git repo might be a clue to that.

Git doesn't support setting file permissions other than the executable
bit, so this is a nonstarter.

Oh, didn't know that, I've certainly know other SCM systems that do.

We could potentially do it with git commit hooks, but the problem is that there's no way to force use of those on clients (a huge deficiency in git, imho).

The best alternative I've been able to come up with is having hooks in a standard location in the repo and then there's one file that people would need to put into their home directory (under ~/.git I think) that would pull all of that stuff in.
--
Jim C. Nasby, Data Architect jim@nasby.net
512.569.9461 (cell) http://jim.nasby.net

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Magnus Hagander
magnus@hagander.net
In reply to: Jim Nasby (#6)
Re: hstore extension version screwup

On Wed, Oct 2, 2013 at 7:17 PM, Jim Nasby <jim@nasby.net> wrote:

On 9/29/13 9:41 PM, Andrew Dunstan wrote:

On 09/29/2013 10:38 PM, Peter Eisentraut wrote:

On Sun, 2013-09-29 at 22:33 -0400, Andrew Dunstan wrote:

Well if these are not meant to be changed then not being able to write
them in your git repo might be a clue to that.

Git doesn't support setting file permissions other than the executable
bit, so this is a nonstarter.

Oh, didn't know that, I've certainly know other SCM systems that do.

We could potentially do it with git commit hooks, but the problem is that
there's no way to force use of those on clients (a huge deficiency in git,
imho).

We could also use git receive hooks, but those would be very hard to
override when you *do* need to modify the files (which you might
within a release).

What we could also do is just have the make all target, or the
configure script, (or something else a developer runs often) chmod the
file. It's not bulletproof in any way, but it would give a decent hint
in most cases.

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Robert Haas
robertmhaas@gmail.com
In reply to: Jim Nasby (#6)
Re: hstore extension version screwup

On Wed, Oct 2, 2013 at 1:17 PM, Jim Nasby <jim@nasby.net> wrote:

On 9/29/13 9:41 PM, Andrew Dunstan wrote:

On 09/29/2013 10:38 PM, Peter Eisentraut wrote:

On Sun, 2013-09-29 at 22:33 -0400, Andrew Dunstan wrote:

Well if these are not meant to be changed then not being able to write
them in your git repo might be a clue to that.

Git doesn't support setting file permissions other than the executable
bit, so this is a nonstarter.

Oh, didn't know that, I've certainly know other SCM systems that do.

We could potentially do it with git commit hooks, but the problem is that
there's no way to force use of those on clients (a huge deficiency in git,
imho).

The best alternative I've been able to come up with is having hooks in a
standard location in the repo and then there's one file that people would
need to put into their home directory (under ~/.git I think) that would pull
all of that stuff in.

ISTM that what we need here is less a git-hook and more of a
regression test, so that if you do the wrong thing, the buildfarm
turns exciting colors. I'm not sure exactly how to write a regression
test for this, but I bet we can dream up something...

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Jim Nasby
jim@nasby.net
In reply to: Magnus Hagander (#7)
Re: hstore extension version screwup

On 10/3/13 12:49 PM, Magnus Hagander wrote:

We could also use git receive hooks, but those would be very hard to
override when you*do* need to modify the files (which you might
within a release).

You can have the receive hook ignore the condition on existence of a file. It's kinda kludgey, but effective. Of course you need to remember to remove the override file when you're done overriding...
--
Jim C. Nasby, Data Architect jim@nasby.net
512.569.9461 (cell) http://jim.nasby.net

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jim Nasby (#9)
Re: hstore extension version screwup

Jim Nasby <jim@nasby.net> writes:

On 10/3/13 12:49 PM, Magnus Hagander wrote:

We could also use git receive hooks, but those would be very hard to
override when you*do* need to modify the files (which you might
within a release).

You can have the receive hook ignore the condition on existence of a file. It's kinda kludgey, but effective. Of course you need to remember to remove the override file when you're done overriding...

An important point here is that we don't want to lock down version m.n
of an extension *until it's shipped in a release*. If we make several
changes in a given extension during a development cycle (some of them
possibly just bugfixes to a previous change), we don't want our tools
forcing us to treat each of those as an upgrade scenario.

This means that any restriction should only apply in the _STABLE branches,
not in master. Not sure how to do that.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers