From b22527f29dba6395a9e950fc655d34914c960f89 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 5 May 2015 15:50:53 -0400 Subject: [PATCH] Fix incorrect declaration of citext's regexp_matches() functions. These functions should return SETOF TEXT[], like the core functions they are wrappers for; but they were incorrectly declared as returning just TEXT[]. This mistake had two results: first, if there was no match you got a scalar null result, whereas what you should get is an empty set (zero rows). Second, the 'g' flag was effectively ignored, since you would get only one result array even if there were multiple matches, as reported by Jeff Certain. While ignoring 'g' is a clear bug, the behavior for no matches might well have been thought to be the intended behavior by people who hadn't compared it carefully to the core regexp_matches() functions. So we should tread carefully about introducing this change in the back branches. Still, it clearly is a bug and so providing some fix is desirable. After discussion, the conclusion was to introduce the change in a 1.1 version of the citext extension (as we would need to do anyway); 1.0 still contains the incorrect behavior. 1.1 is the default and only available version in HEAD, but it is optional in the back branches, where 1.0 remains the default version. People wishing to adopt the fix in back branches will need to explicitly do ALTER EXTENSION citext UPDATE TO '1.1'. (I also provided a downgrade script in the back branches, so people could go back to 1.0 if necessary.) This should be called out as an incompatible change in the 9.5 release notes, although we'll also document it in the next set of back-branch release notes. The notes should mention that any views or rules that use citext's regexp_matches() functions will need to be dropped before upgrading to 1.1, and then recreated again afterwards. Back-patch to 9.1. The bug goes all the way back to citext's introduction in 8.4, but pre-9.1 there is no extension mechanism with which to manage the change. Given the lack of previous complaints it seems unnecessary to change this behavior in 9.0, anyway. --- contrib/citext/Makefile | 2 +- contrib/citext/citext--1.0--1.1.sql | 21 +++++++++++++++++++ .../{citext--1.0.sql => citext--1.1.sql} | 10 ++++----- contrib/citext/citext.control | 2 +- contrib/citext/expected/citext.out | 17 ++++++++++----- contrib/citext/expected/citext_1.out | 17 ++++++++++----- contrib/citext/sql/citext.sql | 4 +++- 7 files changed, 55 insertions(+), 18 deletions(-) create mode 100644 contrib/citext/citext--1.0--1.1.sql rename contrib/citext/{citext--1.0.sql => citext--1.1.sql} (97%) diff --git a/contrib/citext/Makefile b/contrib/citext/Makefile index 267854b5de..61e04bce7a 100644 --- a/contrib/citext/Makefile +++ b/contrib/citext/Makefile @@ -3,7 +3,7 @@ MODULES = citext EXTENSION = citext -DATA = citext--1.0.sql citext--unpackaged--1.0.sql +DATA = citext--1.1.sql citext--1.0--1.1.sql citext--unpackaged--1.0.sql PGFILEDESC = "citext - case-insensitive character string data type" REGRESS = citext diff --git a/contrib/citext/citext--1.0--1.1.sql b/contrib/citext/citext--1.0--1.1.sql new file mode 100644 index 0000000000..e06627e025 --- /dev/null +++ b/contrib/citext/citext--1.0--1.1.sql @@ -0,0 +1,21 @@ +/* contrib/citext/citext--1.0--1.1.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION citext UPDATE TO '1.1'" to load this file. \quit + +/* First we have to remove them from the extension */ +ALTER EXTENSION citext DROP FUNCTION regexp_matches( citext, citext ); +ALTER EXTENSION citext DROP FUNCTION regexp_matches( citext, citext, text ); + +/* Then we can drop them */ +DROP FUNCTION regexp_matches( citext, citext ); +DROP FUNCTION regexp_matches( citext, citext, text ); + +/* Now redefine */ +CREATE FUNCTION regexp_matches( citext, citext ) RETURNS SETOF TEXT[] AS $$ + SELECT pg_catalog.regexp_matches( $1::pg_catalog.text, $2::pg_catalog.text, 'i' ); +$$ LANGUAGE SQL IMMUTABLE STRICT ROWS 1; + +CREATE FUNCTION regexp_matches( citext, citext, text ) RETURNS SETOF TEXT[] AS $$ + SELECT pg_catalog.regexp_matches( $1::pg_catalog.text, $2::pg_catalog.text, CASE WHEN pg_catalog.strpos($3, 'c') = 0 THEN $3 || 'i' ELSE $3 END ); +$$ LANGUAGE SQL IMMUTABLE STRICT ROWS 10; diff --git a/contrib/citext/citext--1.0.sql b/contrib/citext/citext--1.1.sql similarity index 97% rename from contrib/citext/citext--1.0.sql rename to contrib/citext/citext--1.1.sql index 7db2e8d0ae..9ea7c64709 100644 --- a/contrib/citext/citext--1.0.sql +++ b/contrib/citext/citext--1.1.sql @@ -1,4 +1,4 @@ -/* contrib/citext/citext--1.0.sql */ +/* contrib/citext/citext--1.1.sql */ -- complain if script is sourced in psql, rather than via CREATE EXTENSION \echo Use "CREATE EXTENSION citext" to load this file. \quit @@ -440,13 +440,13 @@ CREATE OPERATOR !~~* ( -- XXX TODO Ideally these would be implemented in C. -- -CREATE FUNCTION regexp_matches( citext, citext ) RETURNS TEXT[] AS $$ +CREATE FUNCTION regexp_matches( citext, citext ) RETURNS SETOF TEXT[] AS $$ SELECT pg_catalog.regexp_matches( $1::pg_catalog.text, $2::pg_catalog.text, 'i' ); -$$ LANGUAGE SQL IMMUTABLE STRICT; +$$ LANGUAGE SQL IMMUTABLE STRICT ROWS 1; -CREATE FUNCTION regexp_matches( citext, citext, text ) RETURNS TEXT[] AS $$ +CREATE FUNCTION regexp_matches( citext, citext, text ) RETURNS SETOF TEXT[] AS $$ SELECT pg_catalog.regexp_matches( $1::pg_catalog.text, $2::pg_catalog.text, CASE WHEN pg_catalog.strpos($3, 'c') = 0 THEN $3 || 'i' ELSE $3 END ); -$$ LANGUAGE SQL IMMUTABLE STRICT; +$$ LANGUAGE SQL IMMUTABLE STRICT ROWS 10; CREATE FUNCTION regexp_replace( citext, citext, text ) returns TEXT AS $$ SELECT pg_catalog.regexp_replace( $1::pg_catalog.text, $2::pg_catalog.text, $3, 'i'); diff --git a/contrib/citext/citext.control b/contrib/citext/citext.control index 3eb01a3360..ef90a97bc9 100644 --- a/contrib/citext/citext.control +++ b/contrib/citext/citext.control @@ -1,5 +1,5 @@ # citext extension comment = 'data type for case-insensitive character strings' -default_version = '1.0' +default_version = '1.1' module_pathname = '$libdir/citext' relocatable = true diff --git a/contrib/citext/expected/citext.out b/contrib/citext/expected/citext.out index 411b689b4b..373fe6da54 100644 --- a/contrib/citext/expected/citext.out +++ b/contrib/citext/expected/citext.out @@ -1846,11 +1846,18 @@ SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, ''::cite (1 row) -- c forces case-sensitive -SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, 'c'::citext) = ARRAY[ 'bar', 'beque' ] AS "null"; - null ------- - -(1 row) +SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, 'c'::citext) = ARRAY[ 'bar', 'beque' ] AS "no rows"; + no rows +--------- +(0 rows) + +-- g allows multiple output rows +SELECT regexp_matches('foobarbequebazmorebarbequetoo'::citext, '(BAR)(BEQUE)'::citext, 'g'::citext) AS "two rows"; + two rows +------------- + {bar,beque} + {bar,beque} +(2 rows) SELECT regexp_replace('Thomas'::citext, '.[mN]a.', 'M') = 'ThM' AS t; t diff --git a/contrib/citext/expected/citext_1.out b/contrib/citext/expected/citext_1.out index da3862f49b..fcadd8d392 100644 --- a/contrib/citext/expected/citext_1.out +++ b/contrib/citext/expected/citext_1.out @@ -1846,11 +1846,18 @@ SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, ''::cite (1 row) -- c forces case-sensitive -SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, 'c'::citext) = ARRAY[ 'bar', 'beque' ] AS "null"; - null ------- - -(1 row) +SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, 'c'::citext) = ARRAY[ 'bar', 'beque' ] AS "no rows"; + no rows +--------- +(0 rows) + +-- g allows multiple output rows +SELECT regexp_matches('foobarbequebazmorebarbequetoo'::citext, '(BAR)(BEQUE)'::citext, 'g'::citext) AS "two rows"; + two rows +------------- + {bar,beque} + {bar,beque} +(2 rows) SELECT regexp_replace('Thomas'::citext, '.[mN]a.', 'M') = 'ThM' AS t; t diff --git a/contrib/citext/sql/citext.sql b/contrib/citext/sql/citext.sql index 27678fab5d..950895baea 100644 --- a/contrib/citext/sql/citext.sql +++ b/contrib/citext/sql/citext.sql @@ -601,7 +601,9 @@ SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)', '') = ARRAY[ 'ba SELECT regexp_matches('foobarbequebaz', '(BAR)(BEQUE)'::citext, '') = ARRAY[ 'bar', 'beque' ] AS t; SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, ''::citext) = ARRAY[ 'bar', 'beque' ] AS t; -- c forces case-sensitive -SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, 'c'::citext) = ARRAY[ 'bar', 'beque' ] AS "null"; +SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, 'c'::citext) = ARRAY[ 'bar', 'beque' ] AS "no rows"; +-- g allows multiple output rows +SELECT regexp_matches('foobarbequebazmorebarbequetoo'::citext, '(BAR)(BEQUE)'::citext, 'g'::citext) AS "two rows"; SELECT regexp_replace('Thomas'::citext, '.[mN]a.', 'M') = 'ThM' AS t; SELECT regexp_replace('Thomas'::citext, '.[MN]A.', 'M') = 'ThM' AS t;