Thread: BUG #5781: unaccent() function should be marked IMMUTABLE
BUG #5781: unaccent() function should be marked IMMUTABLE
From
"Grant Hutchins and Peter Jaros"
Date:
The following bug has been logged online:
Bug reference: 5781
Logged by: Grant Hutchins and Peter Jaros
Email address: grant@pivotallabs.com
PostgreSQL version: 9.0.1
Operating system: Mac OS X 10.6.4
Description: unaccent() function should be marked IMMUTABLE
Details:
The unaccent(text) function supplied by contrib/unaccent is marked VOLATILE.
This prevents it from being used in indexes. We believe that the function
meets the requirements to be marked IMMUTABLE.
=# CREATE TABLE foo ();
CREATE TABLE
=# CREATE INDEX bar ON foo ( unaccent('baz') );
ERROR: functions in index expression must be marked IMMUTABLE
=# ALTER FUNCTION unaccent(text) IMMUTABLE;
ALTER FUNCTION
=# CREATE INDEX bar ON foo ( unaccent('baz') );
CREATE INDEX
"Grant Hutchins and Peter Jaros" <grant@pivotallabs.com> writes:
> The unaccent(text) function supplied by contrib/unaccent is marked VOLATILE.
> This prevents it from being used in indexes. We believe that the function
> meets the requirements to be marked IMMUTABLE.
No, it most certainly doesn't. It depends on the behavior of a
dictionary that it has no hard-wired connection to, so the specific
behavior of the dictionary is uncertain. Even if you're willing to
assume that the dictionary being used is the one defined by this
module, that dictionary depends on external configuration files
which are easily changeable.
Arguably it'd be reasonable to change the function's marking from
volatile to stable, but that's not going to be enough to allow use in
indexes.
regards, tom lane
On Fri, Dec 3, 2010 at 4:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Even if you're willing to > assume that the dictionary being used is the one defined by this > module, that dictionary depends on external configuration files > which are easily changeable. Don't we have precedent for this in assuming that things like tsearch dictionaries and parsers and system locales don't change underneath us? -- greg
Greg Stark <gsstark@mit.edu> writes:
> On Fri, Dec 3, 2010 at 4:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Even if you're willing to
>> assume that the dictionary being used is the one defined by this
>> module, that dictionary depends on external configuration files
>> which are easily changeable.
> Don't we have precedent for this in assuming that things like tsearch
> dictionaries and parsers and system locales don't change underneath
> us?
The precedent is to assume such things are stable, not immutable.
Even if you want to assume that, the implementation of unaccent(text) is
such that its results depend on search_path, which is *definitely* not
acceptable for an immutable function. It would need a better way of
tying the function to the dictionary.
regards, tom lane
Tom Lane wrote: > "Grant Hutchins and Peter Jaros" <grant@pivotallabs.com> writes: > > The unaccent(text) function supplied by contrib/unaccent is marked VOLATILE. > > This prevents it from being used in indexes. We believe that the function > > meets the requirements to be marked IMMUTABLE. > > No, it most certainly doesn't. It depends on the behavior of a > dictionary that it has no hard-wired connection to, so the specific > behavior of the dictionary is uncertain. Even if you're willing to > assume that the dictionary being used is the one defined by this > module, that dictionary depends on external configuration files > which are easily changeable. > > Arguably it'd be reasonable to change the function's marking from > volatile to stable, but that's not going to be enough to allow use in > indexes. So, should we change unaccent() from VOLATILE to STABLE? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Wed, Dec 22, 2010 at 8:45 PM, Bruce Momjian <bruce@momjian.us> wrote: > Tom Lane wrote: >> "Grant Hutchins and Peter Jaros" <grant@pivotallabs.com> writes: >> > The unaccent(text) function supplied by contrib/unaccent is marked VOL= ATILE. >> > This prevents it from being used in indexes. We believe that the funct= ion >> > meets the requirements to be marked IMMUTABLE. >> >> No, it most certainly doesn't. =A0It depends on the behavior of a >> dictionary that it has no hard-wired connection to, so the specific >> behavior of the dictionary is uncertain. =A0Even if you're willing to >> assume that the dictionary being used is the one defined by this >> module, that dictionary depends on external configuration files >> which are easily changeable. >> >> Arguably it'd be reasonable to change the function's marking from >> volatile to stable, but that's not going to be enough to allow use in >> indexes. > > So, should we change unaccent() from VOLATILE to STABLE? Sounds like it, but it doesn't sound like it will help much. :-( --=20 Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Wed, Dec 22, 2010 at 8:45 PM, Bruce Momjian <bruce@momjian.us> wrote: > > Tom Lane wrote: > >> "Grant Hutchins and Peter Jaros" <grant@pivotallabs.com> writes: > >> > The unaccent(text) function supplied by contrib/unaccent is marked VOLATILE. > >> > This prevents it from being used in indexes. We believe that the function > >> > meets the requirements to be marked IMMUTABLE. > >> > >> No, it most certainly doesn't. ?It depends on the behavior of a > >> dictionary that it has no hard-wired connection to, so the specific > >> behavior of the dictionary is uncertain. ?Even if you're willing to > >> assume that the dictionary being used is the one defined by this > >> module, that dictionary depends on external configuration files > >> which are easily changeable. > >> > >> Arguably it'd be reasonable to change the function's marking from > >> volatile to stable, but that's not going to be enough to allow use in > >> indexes. > > > > So, should we change unaccent() from VOLATILE to STABLE? > > Sounds like it, but it doesn't sound like it will help much. :-( OK, done, with attached, applied patch. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + diff --git a/contrib/unaccent/unaccent.sql.in b/contrib/unaccent/unaccent.sql.in index 7e397cc..6d712e7 100644 *** /tmp/l3zFae_unaccent.sql.in Mon Dec 27 15:33:55 2010 --- contrib/unaccent/unaccent.sql.in Mon Dec 27 15:24:16 2010 *************** SET search_path = public; *** 6,17 **** CREATE OR REPLACE FUNCTION unaccent(regdictionary, text) RETURNS text AS 'MODULE_PATHNAME', 'unaccent_dict' ! LANGUAGE C STRICT; CREATE OR REPLACE FUNCTION unaccent(text) RETURNS text AS 'MODULE_PATHNAME', 'unaccent_dict' ! LANGUAGE C STRICT; CREATE OR REPLACE FUNCTION unaccent_init(internal) RETURNS internal --- 6,17 ---- CREATE OR REPLACE FUNCTION unaccent(regdictionary, text) RETURNS text AS 'MODULE_PATHNAME', 'unaccent_dict' ! LANGUAGE C STABLE STRICT; CREATE OR REPLACE FUNCTION unaccent(text) RETURNS text AS 'MODULE_PATHNAME', 'unaccent_dict' ! LANGUAGE C STABLE STRICT; CREATE OR REPLACE FUNCTION unaccent_init(internal) RETURNS internal