Обсуждение: contrib loose ends: 9.0 to 9.1 incompatibilities
I've been experimenting with dump/reload of 9.0 contrib-using databases
into 9.1 and then applying CREATE EXTENSION FROM to update the contrib
modules to extension style. There are some cases that fail :-(. Most
of them are caused by the GIN extractQuery API changes. In particular,
a 9.0 dump including intarray will fail altogether with
"ginarrayextract(anyarray, internal) does not exist", and similarly
reloading tsearch2 fails with "gin_extract_tsvector(pg_catalog.tsvector,
internal) does not exist". The functions do still exist in core, but we
added an extra argument to each.
There seem to be two possible solutions to this:
1. We could just revert the pg_proc.h changes so that these two
functions are still shown as taking only 2 arguments. Since GIN doesn't
actually look at the signature claimed in pg_proc, this won't break
anything functionally. It's pretty ugly though, and potentially will
confuse people down the road.
2. We could add extra pg_proc.h entries matching the old signatures.
For the moment these would be stub functions that call the same C code,
though eventually perhaps they could be changed to throw errors.
Preferences anyone?
A related issue is that we similarly changed the signatures of GIN
support functions that properly belong to intarray and tsearch2.
That affects what the "unpackaged" conversion scripts need to expect.
What I'm inclined to do there is just change the scripts to absorb
the old functions as-is without trying to correct their signatures.
Doing otherwise is a bit painful because they are operator class
members, and there's no easy way to unhook them from the opclasses
without dropping the opclasses. The only other fix I can think of
is a direct UPDATE on pg_proc to fix the proargtypes entries, which
would work but seems even uglier.
There are some similar issues in pg_trgm as well. I believe we can fix
these with the available facilities so long as we don't mind the fact
that opclasses upgraded from 9.0 installations will be subtly different
from ones installed fresh in 9.1, for example the new operators being
considered "loose" in the opfamily instead of being bound into an
operator class. While I don't immediately see any problems likely to
arise from that, it's something that could perhaps bite us eventually.
But there's no other answer except embarking on a project to materially
upgrade the capabilities of ALTER OPERATOR CLASS/FAMILY, something I
really don't want to be doing right now.
Comments?
BTW, none of these issues are new with the extensions patch; they are
things we broke awhile ago. I'm thinking it's really past time that
we set up some routine buildfarm-style testing to see if pg_upgrade
from the previous version still works.
regards, tom lane
On Tue, Feb 15, 2011 at 7:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > 1. We could just revert the pg_proc.h changes so that these two > functions are still shown as taking only 2 arguments. Since GIN doesn't > actually look at the signature claimed in pg_proc, this won't break > anything functionally. It's pretty ugly though, and potentially will > confuse people down the road. -1. > 2. We could add extra pg_proc.h entries matching the old signatures. > For the moment these would be stub functions that call the same C code, > though eventually perhaps they could be changed to throw errors. +1. > A related issue is that we similarly changed the signatures of GIN > support functions that properly belong to intarray and tsearch2. > That affects what the "unpackaged" conversion scripts need to expect. > > What I'm inclined to do there is just change the scripts to absorb > the old functions as-is without trying to correct their signatures. > Doing otherwise is a bit painful because they are operator class > members, and there's no easy way to unhook them from the opclasses > without dropping the opclasses. The only other fix I can think of > is a direct UPDATE on pg_proc to fix the proargtypes entries, which > would work but seems even uglier. Hmm. Can we just invent a way to hook them from the opclasses? I have a feeling that now that this extension stuff is in we're going to discover a bunch of these little utility commands that we managed to get by without in the past but now that we're getting more organized about it, we'll need 'em. > There are some similar issues in pg_trgm as well. I believe we can fix > these with the available facilities so long as we don't mind the fact > that opclasses upgraded from 9.0 installations will be subtly different > from ones installed fresh in 9.1, for example the new operators being > considered "loose" in the opfamily instead of being bound into an > operator class. While I don't immediately see any problems likely to > arise from that, it's something that could perhaps bite us eventually. > But there's no other answer except embarking on a project to materially > upgrade the capabilities of ALTER OPERATOR CLASS/FAMILY, something I > really don't want to be doing right now. Or maybe that answers my question. > BTW, none of these issues are new with the extensions patch; they are > things we broke awhile ago. I'm thinking it's really past time that > we set up some routine buildfarm-style testing to see if pg_upgrade > from the previous version still works. Yeah. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Feb 15, 2011 at 7:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 2. We could add extra pg_proc.h entries matching the old signatures.
>> For the moment these would be stub functions that call the same C code,
>> though eventually perhaps they could be changed to throw errors.
> +1.
OK, that's about what I thought too.
>> A related issue is that we similarly changed the signatures of GIN
>> support functions that properly belong to intarray and tsearch2.
>> That affects what the "unpackaged" conversion scripts need to expect.
>>
>> What I'm inclined to do there is just change the scripts to absorb
>> the old functions as-is without trying to correct their signatures.
>> Doing otherwise is a bit painful because they are operator class
>> members, and there's no easy way to unhook them from the opclasses
>> without dropping the opclasses. �The only other fix I can think of
>> is a direct UPDATE on pg_proc to fix the proargtypes entries, which
>> would work but seems even uglier.
> Hmm. Can we just invent a way to hook them from the opclasses? I
> have a feeling that now that this extension stuff is in we're going to
> discover a bunch of these little utility commands that we managed to
> get by without in the past but now that we're getting more organized
> about it, we'll need 'em.
Maybe so. My thought is that extension update scripts are going to be
executed in very well-defined circumstances and it might not be so bad
to let them do direct UPDATEs on the system catalogs instead of writing
lots of special-purpose ALTER commands. In the particular case here,
unhooking, deleting, recreating, and rehooking the functions seems like
way more trouble than it's worth.
>> But there's no other answer except embarking on a project to materially
>> upgrade the capabilities of ALTER OPERATOR CLASS/FAMILY, something I
>> really don't want to be doing right now.
> Or maybe that answers my question.
Yeah. Even granted that we should do it someday, today is not that day.
regards, tom lane
On Wed, Feb 16, 2011 at 11:29 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Hmm. Can we just invent a way to hook them from the opclasses? I >> have a feeling that now that this extension stuff is in we're going to >> discover a bunch of these little utility commands that we managed to >> get by without in the past but now that we're getting more organized >> about it, we'll need 'em. > > Maybe so. My thought is that extension update scripts are going to be > executed in very well-defined circumstances and it might not be so bad > to let them do direct UPDATEs on the system catalogs instead of writing > lots of special-purpose ALTER commands. In the particular case here, > unhooking, deleting, recreating, and rehooking the functions seems like > way more trouble than it's worth. The trouble is that we have no mechanism for conditional logic in upgrade scripts, so if the system catalog structure should change in a way that causes the hook and unhook mechanism to require different logic depending on which PG major version is in use, we're hosed. Or at the very least third-party extension authors are hosed. While I have to hold my nose every time I think about the architecture of pg_upgrade, one thing that it does do for us is provide a significant degree of insulation against problems caused by system catalog changes. Ultimately, the effect of being unable to change on disk format, system catalogs, or other aspects of the system between version is that important and valuable improvements get shot down on compatibility grounds. Inventing mechanisms that protect us from that is important. I invented the extensible EXPLAIN-options syntax not so much because I wanted to be able to get XML output out of it as because I figured that, once we had a way of adding options without fighting with the syntax, people who weren't me would figure out what those options should be and add them. And so it proved: EXPLAIN (BUFFERS) is a very nice feature, much more useful to me personally than EXPLAIN (FORMAT), and I think eventually we'll have more. I don't feel that bad about releasing 9.1 without this unhook-and-rehook machinery because I think we've done pretty well to get this feature as far as we have, and at this point I'm definitely in the camp of wanting a release sooner rather than wanting to squeeze more stuff into it. Having said that, these kinds of problems are impossible to correct without pain if you wait until you're actually backed against the wall. Push will come to shove when someone wants to change the system catalog representation in a way that makes the UPDATE commands used for previous releases no longer work. At that point, regardless of whether you add the unhook-and-rehook mechanism or whether you just modify the UPDATE commands to work with the new structure, you've now imposed a minimum major PG version requirement on that extension that is otherwise unnecessary, or else you now have to distribute two different versions of the extension based on associated major PG release. If we add the unhook-and-rehook mechanism in 9.2, there's a good chance that by the time we need it, a few releases will have gone by and no great pain will result. If not, then at some point there will be a flag day. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Feb 16, 2011 at 12:24 PM, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote: > On 16.02.2011 19:17, Robert Haas wrote: >> >> The trouble is that we have no mechanism for conditional logic in >> upgrade scripts,... > > Can't you put a DO-block there? It's not pretty, but should work.. Tom has repeatedly objected to that solution on the grounds that someone could DROP LANGUAGE plpgsql. In practice, I am sure many third-party extensions will do just that if we don't provide suitable core infrastructure, but not pretty definitely covers it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> The trouble is that we have no mechanism for conditional logic in
> upgrade scripts, so if the system catalog structure should change in a
> way that causes the hook and unhook mechanism to require different
> logic depending on which PG major version is in use, we're hosed.
Well, actually, we *do* have such a mechanism (plpgsql), we just don't
want to use it unless we have to. I wouldn't feel too bad about saying
"upgrading tsearch2 directly from 9.0 to 9.4 requires that you have
plpgsql installed when you issue the CREATE EXTENSION command".
I grant all your points about abstraction being a good thing. But there
are only so many hours in the day, and writing (and then maintaining)
thousands of lines of C code on the grounds that maybe that will let
somebody avoid writing some ugly code someday is not going to get to
the top of my to-do list anytime in the foreseeable future.
regards, tom lane
On 16.02.2011 19:17, Robert Haas wrote: > The trouble is that we have no mechanism for conditional logic in > upgrade scripts,... Can't you put a DO-block there? It's not pretty, but should work.. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Wed, Feb 16, 2011 at 12:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> The trouble is that we have no mechanism for conditional logic in >> upgrade scripts, so if the system catalog structure should change in a >> way that causes the hook and unhook mechanism to require different >> logic depending on which PG major version is in use, we're hosed. > > Well, actually, we *do* have such a mechanism (plpgsql), we just don't > want to use it unless we have to. I wouldn't feel too bad about saying > "upgrading tsearch2 directly from 9.0 to 9.4 requires that you have > plpgsql installed when you issue the CREATE EXTENSION command". > > I grant all your points about abstraction being a good thing. But there > are only so many hours in the day, and writing (and then maintaining) > thousands of lines of C code on the grounds that maybe that will let > somebody avoid writing some ugly code someday is not going to get to > the top of my to-do list anytime in the foreseeable future. Well, it sounds like we're in agreement at least about 9.1, so we can leave the rest of the argument to another day. I *am* surprised that you think it would take *thousands* of lines of code. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> Well, it sounds like we're in agreement at least about 9.1, so we can
> leave the rest of the argument to another day. I *am* surprised that
> you think it would take *thousands* of lines of code.
Well, it all depends on how much ALTER stuff you want to add. An
open-ended commitment to write anything anybody could ever want
would certainly run to thousands of lines. A narrow focus on fixing
proven problems would probably be a lot more manageable ... but if
you have ambitions of fixing problems before anyone has hit them,
it's going to be hard to keep it small.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Well, actually, we *do* have such a mechanism (plpgsql), we just don't > want to use it unless we have to. I wouldn't feel too bad about saying > "upgrading tsearch2 directly from 9.0 to 9.4 requires that you have > plpgsql installed when you issue the CREATE EXTENSION command". We will then need "build"-time requires (build-depends would say debian) so that the system knows what's needed to run the install or upgrade scripts. I've been thinking that's for 9.2, but maybe that would be a simpler fix for you here. Oh and it needs to be able to define plpgsql as such a dependency, so it will probably need to be an extension… Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> Well, actually, we *do* have such a mechanism (plpgsql), we just don't
>> want to use it unless we have to. I wouldn't feel too bad about saying
>> "upgrading tsearch2 directly from 9.0 to 9.4 requires that you have
>> plpgsql installed when you issue the CREATE EXTENSION command".
> We will then need "build"-time requires (build-depends would say debian)
> so that the system knows what's needed to run the install or upgrade
> scripts.
Huh? It's still an install-time dependency, and anyway the failure
would be pretty obvious. Please let us not spend time garnishing this
facility with unnecessary stuff, when the list of actually *important*
missing stuff is still a mile long. Think about what's needed to
support procedural languages as extensions, for starters. I'll give you
a hint: none of it has anything whatever to do with copying features
from RPM or Debian.
regards, tom lane
On Feb 16, 2011, at 1:20 PM, Dimitri Fontaine wrote: > We will then need "build"-time requires (build-depends would say debian) > so that the system knows what's needed to run the install or upgrade > scripts. I've been thinking that's for 9.2, but maybe that would be a > simpler fix for you here. > > Oh and it needs to be able to define plpgsql as such a dependency, so it > will probably need to be an extension… With a version number… David
So, after some testing, attached are two different fixed-up versions of
pg_tgrm's update-from-unpackaged script. The first one leaves the
parameter lists of some GIN support functions different from what they
would be if you installed pg_trgrm fresh in 9.1. The second one fixes
the parameter lists too, by means of really ugly direct UPDATEs on
pg_proc. I'm unsure which one to apply --- any opinions?
It's worth noting that both versions still leave the pg_trgm opclasses a
bit different from a fresh install, because the added operators are
"loose" in the opfamily rather than being bound into the opclass. This
hasn't got any real functional effect, but if you were feeling paranoid
you could worry about whether the two different states could cause
problems for future versions of the update script. As far as I can see,
the only thing we could realistically do about this with the tools at
hand is to change pg_trgm's install script so that it also creates the
new-in-9.1 entries "loose". That seems a tad ugly, but depending on
where you stand on the paranoia scale you might think it's a good idea.
There is definitely no point in that refinement unless we update the
function parameter lists, though.
Comments?
regards, tom lane
/* contrib/pg_trgm/pg_trgm--unpackaged--1.0.sql */
ALTER EXTENSION pg_trgm ADD function set_limit(real);
ALTER EXTENSION pg_trgm ADD function show_limit();
ALTER EXTENSION pg_trgm ADD function show_trgm(text);
ALTER EXTENSION pg_trgm ADD function similarity(text,text);
ALTER EXTENSION pg_trgm ADD function similarity_op(text,text);
ALTER EXTENSION pg_trgm ADD operator %(text,text);
ALTER EXTENSION pg_trgm ADD type gtrgm;
ALTER EXTENSION pg_trgm ADD function gtrgm_in(cstring);
ALTER EXTENSION pg_trgm ADD function gtrgm_out(gtrgm);
ALTER EXTENSION pg_trgm ADD function gtrgm_consistent(internal,text,integer,oid,internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_compress(internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_decompress(internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_penalty(internal,internal,internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_picksplit(internal,internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_union(bytea,internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_same(gtrgm,gtrgm,internal);
ALTER EXTENSION pg_trgm ADD operator family gist_trgm_ops using gist;
ALTER EXTENSION pg_trgm ADD operator class gist_trgm_ops using gist;
ALTER EXTENSION pg_trgm ADD operator family gin_trgm_ops using gin;
ALTER EXTENSION pg_trgm ADD operator class gin_trgm_ops using gin;
-- These functions had different names/signatures in 9.0. We can't just
-- drop and recreate them because they are linked into the GIN opclass,
-- so we need some hacks.
-- First, absorb them into the extension under their old names.
ALTER EXTENSION pg_trgm ADD function gin_extract_trgm(text, internal);
ALTER EXTENSION pg_trgm ADD function gin_extract_trgm(text, internal, int2, internal, internal);
ALTER EXTENSION pg_trgm ADD function gin_trgm_consistent(internal,smallint,text,integer,internal,internal);
-- Fix the names, and then do CREATE OR REPLACE to adjust the function
-- bodies to be correct (ie, reference the correct C symbol). We do not
-- attempt to change the parameter lists, however. It's not necessary
-- since GIN doesn't care, and there's no clean way to do it.
ALTER FUNCTION gin_extract_trgm(text, internal)
RENAME TO gin_extract_value_trgm;
CREATE OR REPLACE FUNCTION gin_extract_value_trgm(text, internal)
RETURNS internal
AS 'MODULE_PATHNAME'
LANGUAGE C IMMUTABLE STRICT;
ALTER FUNCTION gin_extract_trgm(text, internal, int2, internal, internal)
RENAME TO gin_extract_query_trgm;
CREATE OR REPLACE FUNCTION gin_extract_query_trgm(text, internal, int2, internal, internal)
RETURNS internal
AS 'MODULE_PATHNAME'
LANGUAGE C IMMUTABLE STRICT;
-- gin_trgm_consistent didn't change name, so nothing more to do for it.
-- These were not in 9.0:
CREATE FUNCTION similarity_dist(text,text)
RETURNS float4
AS 'MODULE_PATHNAME'
LANGUAGE C STRICT IMMUTABLE;
CREATE OPERATOR <-> (
LEFTARG = text,
RIGHTARG = text,
PROCEDURE = similarity_dist,
COMMUTATOR = '<->'
);
CREATE FUNCTION gtrgm_distance(internal,text,int,oid)
RETURNS float8
AS 'MODULE_PATHNAME'
LANGUAGE C IMMUTABLE STRICT;
-- Add new stuff to the operator classes. Note this will result in the
-- added stuff being "loose" in the operator family, rather than bound
-- into the operator class as it would be when creating the extension
-- from scratch. That shouldn't be problematic.
ALTER OPERATOR FAMILY gist_trgm_ops USING gist ADD
OPERATOR 2 <-> (text, text) FOR ORDER BY pg_catalog.float_ops,
OPERATOR 3 pg_catalog.~~ (text, text),
OPERATOR 4 pg_catalog.~~* (text, text),
FUNCTION 8 (text, text) gtrgm_distance (internal, text, int, oid);
ALTER OPERATOR FAMILY gin_trgm_ops USING gin ADD
OPERATOR 3 pg_catalog.~~ (text, text),
OPERATOR 4 pg_catalog.~~* (text, text);
/* contrib/pg_trgm/pg_trgm--unpackaged--1.0.sql */
ALTER EXTENSION pg_trgm ADD function set_limit(real);
ALTER EXTENSION pg_trgm ADD function show_limit();
ALTER EXTENSION pg_trgm ADD function show_trgm(text);
ALTER EXTENSION pg_trgm ADD function similarity(text,text);
ALTER EXTENSION pg_trgm ADD function similarity_op(text,text);
ALTER EXTENSION pg_trgm ADD operator %(text,text);
ALTER EXTENSION pg_trgm ADD type gtrgm;
ALTER EXTENSION pg_trgm ADD function gtrgm_in(cstring);
ALTER EXTENSION pg_trgm ADD function gtrgm_out(gtrgm);
ALTER EXTENSION pg_trgm ADD function gtrgm_consistent(internal,text,integer,oid,internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_compress(internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_decompress(internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_penalty(internal,internal,internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_picksplit(internal,internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_union(bytea,internal);
ALTER EXTENSION pg_trgm ADD function gtrgm_same(gtrgm,gtrgm,internal);
ALTER EXTENSION pg_trgm ADD operator family gist_trgm_ops using gist;
ALTER EXTENSION pg_trgm ADD operator class gist_trgm_ops using gist;
ALTER EXTENSION pg_trgm ADD operator family gin_trgm_ops using gin;
ALTER EXTENSION pg_trgm ADD operator class gin_trgm_ops using gin;
-- These functions had different names/signatures in 9.0. We can't just
-- drop and recreate them because they are linked into the GIN opclass,
-- so we need some ugly hacks.
-- First, absorb them into the extension under their old names.
ALTER EXTENSION pg_trgm ADD function gin_extract_trgm(text, internal);
ALTER EXTENSION pg_trgm ADD function gin_extract_trgm(text, internal, int2, internal, internal);
ALTER EXTENSION pg_trgm ADD function gin_trgm_consistent(internal,smallint,text,integer,internal,internal);
-- Fix the names, and then do CREATE OR REPLACE to adjust the function
-- bodies to be correct (ie, reference the correct C symbol).
ALTER FUNCTION gin_extract_trgm(text, internal)
RENAME TO gin_extract_value_trgm;
CREATE OR REPLACE FUNCTION gin_extract_value_trgm(text, internal)
RETURNS internal
AS 'MODULE_PATHNAME'
LANGUAGE C IMMUTABLE STRICT;
ALTER FUNCTION gin_extract_trgm(text, internal, int2, internal, internal)
RENAME TO gin_extract_query_trgm;
CREATE OR REPLACE FUNCTION gin_extract_query_trgm(text, internal, int2, internal, internal)
RETURNS internal
AS 'MODULE_PATHNAME'
LANGUAGE C IMMUTABLE STRICT;
-- gin_trgm_consistent didn't change name.
-- Last, fix the parameter lists by means of direct UPDATE on the pg_proc
-- entries. This is ugly as can be, but there's no other way to do it
-- while preserving the identities (OIDs) of the functions.
UPDATE pg_catalog.pg_proc
SET pronargs = 7, proargtypes = '25 2281 21 2281 2281 2281 2281'
WHERE oid = 'gin_extract_query_trgm(text,internal,int2,internal,internal)'::pg_catalog.regprocedure;
UPDATE pg_catalog.pg_proc
SET pronargs = 8, proargtypes = '2281 21 25 23 2281 2281 2281 2281'
WHERE oid = 'gin_trgm_consistent(internal,smallint,text,integer,internal,internal)'::pg_catalog.regprocedure;
-- These were not in 9.0:
CREATE FUNCTION similarity_dist(text,text)
RETURNS float4
AS 'MODULE_PATHNAME'
LANGUAGE C STRICT IMMUTABLE;
CREATE OPERATOR <-> (
LEFTARG = text,
RIGHTARG = text,
PROCEDURE = similarity_dist,
COMMUTATOR = '<->'
);
CREATE FUNCTION gtrgm_distance(internal,text,int,oid)
RETURNS float8
AS 'MODULE_PATHNAME'
LANGUAGE C IMMUTABLE STRICT;
-- Add new stuff to the operator classes. Note this will result in the
-- added stuff being "loose" in the operator family, rather than bound
-- into the operator class as it would be when creating the extension
-- from scratch. That shouldn't be problematic.
ALTER OPERATOR FAMILY gist_trgm_ops USING gist ADD
OPERATOR 2 <-> (text, text) FOR ORDER BY pg_catalog.float_ops,
OPERATOR 3 pg_catalog.~~ (text, text),
OPERATOR 4 pg_catalog.~~* (text, text),
FUNCTION 8 (text, text) gtrgm_distance (internal, text, int, oid);
ALTER OPERATOR FAMILY gin_trgm_ops USING gin ADD
OPERATOR 3 pg_catalog.~~ (text, text),
OPERATOR 4 pg_catalog.~~* (text, text);
On Thu, Feb 17, 2011 at 12:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > So, after some testing, attached are two different fixed-up versions of > pg_tgrm's update-from-unpackaged script. The first one leaves the > parameter lists of some GIN support functions different from what they > would be if you installed pg_trgrm fresh in 9.1. The second one fixes > the parameter lists too, by means of really ugly direct UPDATEs on > pg_proc. I'm unsure which one to apply --- any opinions? > > It's worth noting that both versions still leave the pg_trgm opclasses a > bit different from a fresh install, because the added operators are > "loose" in the opfamily rather than being bound into the opclass. This > hasn't got any real functional effect, but if you were feeling paranoid > you could worry about whether the two different states could cause > problems for future versions of the update script. As far as I can see, > the only thing we could realistically do about this with the tools at > hand is to change pg_trgm's install script so that it also creates the > new-in-9.1 entries "loose". That seems a tad ugly, but depending on > where you stand on the paranoia scale you might think it's a good idea. > There is definitely no point in that refinement unless we update the > function parameter lists, though. > > Comments? I think we should try to make the state match as closely as possible, no matter how you got there. Otherwise, I think we're storing up a host of future pain for ourselves. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Feb 17, 2011 at 12:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It's worth noting that both versions still leave the pg_trgm opclasses a
>> bit different from a fresh install, because the added operators are
>> "loose" in the opfamily rather than being bound into the opclass. �This
>> hasn't got any real functional effect, but if you were feeling paranoid
>> you could worry about whether the two different states could cause
>> problems for future versions of the update script. �As far as I can see,
>> the only thing we could realistically do about this with the tools at
>> hand is to change pg_trgm's install script so that it also creates the
>> new-in-9.1 entries "loose". �That seems a tad ugly, but depending on
>> where you stand on the paranoia scale you might think it's a good idea.
>> There is definitely no point in that refinement unless we update the
>> function parameter lists, though.
>>
>> Comments?
> I think we should try to make the state match as closely as possible,
> no matter how you got there. Otherwise, I think we're storing up a
> host of future pain for ourselves.
Well, if you're willing to hold your nose for the "UPDATE pg_proc" hack,
we can make it so.
regards, tom lane
On Thu, Feb 17, 2011 at 1:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Thu, Feb 17, 2011 at 12:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> It's worth noting that both versions still leave the pg_trgm opclasses a >>> bit different from a fresh install, because the added operators are >>> "loose" in the opfamily rather than being bound into the opclass. This >>> hasn't got any real functional effect, but if you were feeling paranoid >>> you could worry about whether the two different states could cause >>> problems for future versions of the update script. As far as I can see, >>> the only thing we could realistically do about this with the tools at >>> hand is to change pg_trgm's install script so that it also creates the >>> new-in-9.1 entries "loose". That seems a tad ugly, but depending on >>> where you stand on the paranoia scale you might think it's a good idea. >>> There is definitely no point in that refinement unless we update the >>> function parameter lists, though. >>> >>> Comments? > >> I think we should try to make the state match as closely as possible, >> no matter how you got there. Otherwise, I think we're storing up a >> host of future pain for ourselves. > > Well, if you're willing to hold your nose for the "UPDATE pg_proc" hack, > we can make it so. Yes, I think that's better than leaving things in a different state. It's not my first choice, but it's better than the alternative. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> I think we should try to make the state match as closely as possible,
>> no matter how you got there. Otherwise, I think we're storing up a
>> host of future pain for ourselves.
> Well, if you're willing to hold your nose for the "UPDATE pg_proc" hack,
> we can make it so.
I believe I've now fixed all the discrepancies between fresh installs
and 9.0 updates of contrib modules, except for these:
1. citext COLLATABLE option (see adjacent thread)
2. intarray and tsearch2 use some core support functions in their
GIN opclasses, and those support functions changed signatures in 9.1.
The current solution to this involves having stub functions in core
with the old signatures; when you do an upgrade from the 9.0 version
of one of these contrib modules, its opclass will be pointing at the
stub version instead of the preferred version. I guess we could fix
that with a direct UPDATE on pg_amproc but I'm not sure that's a
good idea. Note these functions aren't actually *members* of the
extensions, just things it references, so the odds of future trouble
seem pretty small. On the other hand, if we don't do this, it's
unclear when we'll ever be able to get rid of the stubs.
Comments?
regards, tom lane
On Thu, Feb 17, 2011 at 9:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I wrote: >> Robert Haas <robertmhaas@gmail.com> writes: >>> I think we should try to make the state match as closely as possible, >>> no matter how you got there. Otherwise, I think we're storing up a >>> host of future pain for ourselves. > >> Well, if you're willing to hold your nose for the "UPDATE pg_proc" hack, >> we can make it so. > > I believe I've now fixed all the discrepancies between fresh installs > and 9.0 updates of contrib modules, except for these: > > 1. citext COLLATABLE option (see adjacent thread) > > 2. intarray and tsearch2 use some core support functions in their > GIN opclasses, and those support functions changed signatures in 9.1. > The current solution to this involves having stub functions in core > with the old signatures; when you do an upgrade from the 9.0 version > of one of these contrib modules, its opclass will be pointing at the > stub version instead of the preferred version. I guess we could fix > that with a direct UPDATE on pg_amproc but I'm not sure that's a > good idea. Note these functions aren't actually *members* of the > extensions, just things it references, so the odds of future trouble > seem pretty small. On the other hand, if we don't do this, it's > unclear when we'll ever be able to get rid of the stubs. > > Comments? ISTM that the pg_amproc entries are part of the operator class, which is owned by the extension. So it's the upgrade script's job to leave the operator class in the right state. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Feb 17, 2011 at 9:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 2. intarray and tsearch2 use some core support functions in their
>> GIN opclasses, and those support functions changed signatures in 9.1.
>> The current solution to this involves having stub functions in core
>> with the old signatures; when you do an upgrade from the 9.0 version
>> of one of these contrib modules, its opclass will be pointing at the
>> stub version instead of the preferred version. �I guess we could fix
>> that with a direct UPDATE on pg_amproc but I'm not sure that's a
>> good idea. �Note these functions aren't actually *members* of the
>> extensions, just things it references, so the odds of future trouble
>> seem pretty small. �On the other hand, if we don't do this, it's
>> unclear when we'll ever be able to get rid of the stubs.
>>
>> Comments?
> ISTM that the pg_amproc entries are part of the operator class, which
> is owned by the extension. So it's the upgrade script's job to leave
> the operator class in the right state.
OK, I held my nose and inserted UPDATE commands to make the opclasses
match. AFAICT the only remaining discrepancy between contrib modules
made fresh in 9.1 and those updated from 9.0 is the question of citext's
collation property, which as noted in the other thread is not worth
dealing with until the collation stuff is a bit better thought out.
regards, tom lane
On Fri, Feb 18, 2011 at 12:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Thu, Feb 17, 2011 at 9:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> 2. intarray and tsearch2 use some core support functions in their >>> GIN opclasses, and those support functions changed signatures in 9.1. >>> The current solution to this involves having stub functions in core >>> with the old signatures; when you do an upgrade from the 9.0 version >>> of one of these contrib modules, its opclass will be pointing at the >>> stub version instead of the preferred version. I guess we could fix >>> that with a direct UPDATE on pg_amproc but I'm not sure that's a >>> good idea. Note these functions aren't actually *members* of the >>> extensions, just things it references, so the odds of future trouble >>> seem pretty small. On the other hand, if we don't do this, it's >>> unclear when we'll ever be able to get rid of the stubs. >>> >>> Comments? > >> ISTM that the pg_amproc entries are part of the operator class, which >> is owned by the extension. So it's the upgrade script's job to leave >> the operator class in the right state. > > OK, I held my nose and inserted UPDATE commands to make the opclasses > match. AFAICT the only remaining discrepancy between contrib modules > made fresh in 9.1 and those updated from 9.0 is the question of citext's > collation property, which as noted in the other thread is not worth > dealing with until the collation stuff is a bit better thought out. OK. Thanks for nailing all of this down - that's got to have been a heck of a job. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Feb 18, 2011 at 12:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> OK, I held my nose and inserted UPDATE commands to make the opclasses
>> match. �AFAICT the only remaining discrepancy between contrib modules
>> made fresh in 9.1 and those updated from 9.0 is the question of citext's
>> collation property, which as noted in the other thread is not worth
>> dealing with until the collation stuff is a bit better thought out.
> OK. Thanks for nailing all of this down - that's got to have been a
> heck of a job.
Yeah, it was a bit of a pain, and took longer than I would've hoped.
It was worth doing though --- I think it not unlikely that in the
long run, the extensions feature will be seen as the largest single
improvement in 9.1.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: >> OK. Thanks for nailing all of this down - that's got to have been a >> heck of a job. +1 > Yeah, it was a bit of a pain, and took longer than I would've hoped. Well, with some luck (and effort) 9.2 will have the missing DDL pieces. I think the extension features means we now need support for all kind of ALTER things, even on operator classes and families. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support