Re: Fixing busted citext function declarations

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: Fixing busted citext function declarations
Date: 2015-05-05 17:07:08
Message-ID: 20150505170708.GK2523@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David E. Wheeler 2015-05-05 17:17:59 Re: Fixing busted citext function declarations
Previous Message Robert Haas 2015-05-05 17:05:38 Re: parallel mode and parallel contexts