Re: Fixing busted citext function declarations - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: Fixing busted citext function declarations
Date
Msg-id 20150505170708.GK2523@alvh.no-ip.org
Whole thread Raw
In response to Fixing busted citext function declarations  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Fixing busted citext function declarations
Re: Fixing busted citext function declarations
Re: Fixing busted citext function declarations
List pgsql-hackers
Tom Lane wrote:

> * We can't use CREATE OR REPLACE FUNCTION in the upgrade script because
> that intentionally doesn't let you change the result type of an existing
> function.  I considered doing a manual UPDATE of the pg_proc entry, but
> then remembered why CREATE OR REPLACE FUNCTION is picky about this: the
> result type, including set-ness, is embedded in the parse tree of any view
> referencing the function.  So AFAICS we need to actually drop and recreate
> the citext regexp_matches() functions in the upgrade script.  That means
> "ALTER EXTENSION citext UPDATE" will fail if these functions are being
> used in any views.  That's annoying but I see no way around it.  (We
> could have the upgrade script do DROP CASCADE, but that seems way too
> destructive.)

I think we do need to have the upgrade script drop/recreate without
cascade.  Then, users can "alter extension upgrade", note the
problematic views (which should be part of the error message), drop
them, then retry the extension update and re-create their views.  This
is necessarily a manual procedure -- I don't think we can re-create
views using the function automatically.  CASCADE seems pretty dangerous.

(I think it is possible that the behavior change is actually problematic
as opposed to just behaving differently.  For instance, if the function
is used in a subselect that's expected to return only one row, and it
suddenly starts returning more, the query would raise an error.  Seems
better to err on the side of caution.)

> * Is anyone concerned about back-patching this fix?  I suppose you could
> make an argument that some app somewhere might be relying on the current
> behavior of citext regexp_matches(), which effectively is to return only
> the first match even if you said 'g'.  One answer would be to keep on
> supplying the citext 1.0 script in the back branches, so that anyone who
> really needed it could still install 1.0 not 1.1.  That's not our usual
> practice though, so unless there's serious concern that such a problem
> really exists, I'd rather not do that.

I think we should keep the 1.0 version this time, in back branches.
This can be invoked manually only (i.e. the default version would still
be 1.1), and it wouldn't be provided in the master branch, so it would
not cause long-term maintenance pain.  But it should be possible for
people to re-create their current databases, in case they need to
upgrade for unrelated reasons and have views dependant on this behavior.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: parallel mode and parallel contexts
Next
From: "David E. Wheeler"
Date:
Subject: Re: Fixing busted citext function declarations