Re: PATCH: CITEXT 2.0 v4

Howdy,

I've attached a new patch with the latest revisions of for the citext
contrib module patch. The changes include:

* Using strlen() to pass string lengths to the comparison function,
since lowercasing the value can change the length. Per Tom Lane.
* Made citextcmp consistently return int32, per Tom Lane.
* Made the hash index function return the proper value, per Tom Lane.
* Removed the COMMENTs and GRANTs from citext.sql.in.
* Added a cast function from bpchar to citext, as suggested by Tom Lane.
* Set the storage type for CITEXT to "extended", to ensure that it will
be toastable. Per Tom Lane.
* Fixed the COMMUTATOR of <=.
* Changed the cast from citext to bpchar from implicit to assignment.
This eliminates ambiguous function resolutions.
* Eliminated superflous functions, per Tom Lane.
* Removed unnecessary `OPERATOR()` calls in NEGATORs and the like.
* Added binary in/out functions. Per Tom Lane
* Added an explicit shell type to make the output a bit quieter.
* Converted tests to pure SQL and omitted multibyte tests (though a
few remain commented-out).
* Reorganized and expanded the documentation a bit.

This version is far better than I started with, and I'm very grateful
for the feedback.

Now, I have a few remaining questions to ask, mostly just to get your
opinions:

* The README for citext 1.0 on pgFoundry says:

> I had to make a decision on casting between types for regular
> expressions and
> decided that if any parameter is of citext type then case
> insensitive applies.
> For example applying regular expressions with a varchar and a citext
> will
> produce a case-insensitive result.
>
> Having thought about this afterwards I realised that since we have
> the option
> to use case-insensitive results with regular expressions I should
> have left the
> behaviour exactly as text and then you have the best of both
> worlds... oh well
> not hard to change for any of you perfectionists!

I followed the original and made all the regex and LIKE comparisons
case-insensitive. But maybe I should not have? Especially since the
regular expression functions (e.g., regexp_replace()) and a few non-
regex functions (e.g., replace()) still don't behave case-insensitively?

* If the answer is "no", how can I make those functions behave case-
insensitively? (See the "TODO" tests.)

* Should there be any other casts? To and from name, perhaps?

Thanks!

David

On Jul 15, 2008, at 22:23, David E. Wheeler wrote:

> * The README for citext 1.0 on pgFoundry says:
>
>> I had to make a decision on casting between types for regular
>> expressions and
>> decided that if any parameter is of citext type then case
>> insensitive applies.
>> For example applying regular expressions with a varchar and a
>> citext will
>> produce a case-insensitive result.
>>
>> Having thought about this afterwards I realised that since we have
>> the option
>> to use case-insensitive results with regular expressions I should
>> have left the
>> behaviour exactly as text and then you have the best of both
>> worlds... oh well
>> not hard to change for any of you perfectionists!
>
> I followed the original and made all the regex and LIKE comparisons
> case-insensitive. But maybe I should not have? Especially since the
> regular expression functions (e.g., regexp_replace()) and a few non-
> regex functions (e.g., replace()) still don't behave case-
> insensitively?

I was thinking about this a bit last night and wanted to fill things
out a bit.

As a programmer, I find Donald Fraser's hindsight to be more
appealing, because at least that way I have the option to do matching
against CITEXT strings case-sensitively when I want to.

OTOH, if what we want is to have CITEXT work more like a case-
insensitive collation, then the expectation from the matching
operators and functions might be different. Does anyone have any idea
whether regex and LIKE matching against a string in a case-insensitive
collation would match case-insensitively or not? If so, then maybe the
regex and LIKE ops and funcs *should* match case-insensitively? If
not, or if only for some collations, then I would think not.

Either way, I know of no way, currently, to allow functions like
replace(), split_part(), strpos(), and translate() to match case-
insensitiely, even if we wanted to. Anyone have any ideas?

> * If the answer is "no", how can I make those functions behave case-
> insensitively? (See the "TODO" tests.)
>
> * Should there be any other casts? To and from name, perhaps?

Thanks,

David

On Wednesday 16 July 2008 13:54:25 David E. Wheeler wrote:
> On Jul 15, 2008, at 22:23, David E. Wheeler wrote:
> > * The README for citext 1.0 on pgFoundry says:
> >> I had to make a decision on casting between types for regular
> >> expressions and
> >> decided that if any parameter is of citext type then case
> >> insensitive applies.
> >> For example applying regular expressions with a varchar and a
> >> citext will
> >> produce a case-insensitive result.
> >>
> >> Having thought about this afterwards I realised that since we have
> >> the option
> >> to use case-insensitive results with regular expressions I should
> >> have left the
> >> behaviour exactly as text and then you have the best of both
> >> worlds... oh well
> >> not hard to change for any of you perfectionists!
> >
> > I followed the original and made all the regex and LIKE comparisons
> > case-insensitive. But maybe I should not have? Especially since the
> > regular expression functions (e.g., regexp_replace()) and a few non-
> > regex functions (e.g., replace()) still don't behave case-
> > insensitively?
>
> I was thinking about this a bit last night and wanted to fill things
> out a bit.
>
> As a programmer, I find Donald Fraser's hindsight to be more
> appealing, because at least that way I have the option to do matching
> against CITEXT strings case-sensitively when I want to.
>
> OTOH, if what we want is to have CITEXT work more like a case-
> insensitive collation, then the expectation from the matching
> operators and functions might be different. Does anyone have any idea
> whether regex and LIKE matching against a string in a case-insensitive
> collation would match case-insensitively or not? If so, then maybe the
> regex and LIKE ops and funcs *should* match case-insensitively? If
> not, or if only for some collations, then I would think not.
>
> Either way, I know of no way, currently, to allow functions like
> replace(), split_part(), strpos(), and translate() to match case-
> insensitiely, even if we wanted to. Anyone have any ideas?
>
> > * If the answer is "no", how can I make those functions behave case-
> > insensitively? (See the "TODO" tests.)
> >
> > * Should there be any other casts? To and from name, perhaps?
>

AIUI, your propsing the following:

select 'x'::citext = 'X'::citext;
?column?
----------
t
(1 row)

select 'x'::citext ~ 'X'::citext;
?column?
----------
f
(1 row)

I understand the desire for flexibility, but the above seems wierd to me.

--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

On Jul 16, 2008, at 11:20, Robert Treat wrote:

>> I was thinking about this a bit last night and wanted to fill things
>> out a bit.
>>
>> As a programmer, I find Donald Fraser's hindsight to be more
>> appealing, because at least that way I have the option to do matching
>> against CITEXT strings case-sensitively when I want to.
>>
>> OTOH, if what we want is to have CITEXT work more like a case-
>> insensitive collation, then the expectation from the matching
>> operators and functions might be different. Does anyone have any idea
>> whether regex and LIKE matching against a string in a case-
>> insensitive
>> collation would match case-insensitively or not? If so, then maybe
>> the
>> regex and LIKE ops and funcs *should* match case-insensitively? If
>> not, or if only for some collations, then I would think not.
>>
>> Either way, I know of no way, currently, to allow functions like
>> replace(), split_part(), strpos(), and translate() to match case-
>> insensitiely, even if we wanted to. Anyone have any ideas?
>>
>>> * If the answer is "no", how can I make those functions behave case-
>>> insensitively? (See the "TODO" tests.)
>>>
>>> * Should there be any other casts? To and from name, perhaps?
>>
>
> AIUI, your propsing the following:
>
> select 'x'::citext = 'X'::citext;
> ?column?
> ----------
> t
> (1 row)
>
> select 'x'::citext ~ 'X'::citext;
> ?column?
> ----------
> f
> (1 row)
>
> I understand the desire for flexibility, but the above seems wierd
> to me.

That's what Donald Fraser suggested, and I see some value in that, but
wanted to get some other opinions. And you're right, that does seem a
bit weird.

The trouble is that, right now:

template1=# select regexp_replace( 'fxx'::citext, 'X'::citext, 'o');
regexp_replace
----------------
fxx
(1 row)

So there's an inconsistency there. I don't know how to make that work
case-insensitively.

Best,

David

Am 16.07.2008 um 20:38 schrieb David E. Wheeler:
>
> The trouble is that, right now:
>
> template1=# select regexp_replace( 'fxx'::citext, 'X'::citext, 'o');
> regexp_replace
> ----------------
> fxx
> (1 row)
>
> So there's an inconsistency there. I don't know how to make that
> work case-insensitively.

Wouldn't it be possible to create a variant of regexp_replace, i.e.
regexp_replace(citext,citext,text), which would again lower-case the
first two arguments before passing the input to
regexp_replace(text,text,text)?

Best Regards
Michael Paesold

On Jul 17, 2008, at 03:45, Michael Paesold wrote:

> Wouldn't it be possible to create a variant of regexp_replace, i.e.
> regexp_replace(citext,citext,text), which would again lower-case the
> first two arguments before passing the input to
> regexp_replace(text,text,text)?

Sure, but then you end up with this:

template1=# select regexp_replace( 'Fxx'::citext, 'X'::citext, 'o');
regexp_replace
----------------
foo
(1 row)

Which is just wrong. I'm going to look at the regex C functions today
and see if there's an easy way to just always pass them the 'i' flag,
which would do the trick. That still won't help replace(),
split_part(), or translate(), however.

Best,

David

David E. Wheeler writes:

> On Jul 17, 2008, at 03:45, Michael Paesold wrote:
>
>> Wouldn't it be possible to create a variant of regexp_replace, i.e.
>> regexp_replace(citext,citext,text), which would again lower-case
>> the first two arguments before passing the input to
>> regexp_replace(text,text,text)?
>
> Sure, but then you end up with this:
>
> template1=# select regexp_replace( 'Fxx'::citext, 'X'::citext, 'o');
> regexp_replace
> ----------------
> foo
> (1 row)

Yeah, you are right, I see. :-)

> Which is just wrong. I'm going to look at the regex C functions
> today and see if there's an easy way to just always pass them the
> 'i' flag, which would do the trick. That still won't help replace(),
> split_part(), or translate(), however.

Calling regex functions with the case-insensitivity option would be
great. It should also be possible to rewrite replace() into
regexp_replace() by first escaping the regex meta characters.

Actually re-implementing those functions in a case insensitive way
would still be an option, but of course some amount of work. The
question is, how much use case there is.

Best Regards
Michael Paesold

On Jul 18, 2008, at 01:39, Michael Paesold wrote:

> Calling regex functions with the case-insensitivity option would be
> great. It should also be possible to rewrite replace() into
> regexp_replace() by first escaping the regex meta characters.
>
> Actually re-implementing those functions in a case insensitive way
> would still be an option, but of course some amount of work. The
> question is, how much use case there is.

Not much for me. I might use the regex functions, but would be happy
to manually pass the "i" flag.

However, if someone with a lot more C and Pg core knowledge wanted to
sit down with me for a couple hours next week and help me bang out
these functions, that would be great. I'd love to have the
implementation be that much more complete.

I do believe that, as it stands now in the v4 patch, citext is pretty
close to ready, and certainly commit-able.

Thanks,

David

On Jul 18, 2008, at 09:53, David E. Wheeler wrote:

> However, if someone with a lot more C and Pg core knowledge wanted
> to sit down with me for a couple hours next week and help me bang
> out these functions, that would be great. I'd love to have the
> implementation be that much more complete.

I've implemented fixes for the regexp_* functions and strpos() in pure
SQL, like so:

CREATE OR REPLACE FUNCTION regexp_matches( citext, citext ) RETURNS
TEXT[] AS '
SELECT regexp_matches( $1::text, $2::text, ''i'' );
' LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION regexp_matches( citext, citext, text )
RETURNS TEXT[] AS '
SELECT regexp_matches( $1::text, $2::text, CASE WHEN strpos($3,
''c'') = 0 THEN $3 || ''i'' ELSE $3 END );
' LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION regexp_replace( citext, citext, text )
returns TEXT AS '
SELECT regexp_replace( $1::text, $2::text, $3, ''i'');
' LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION regexp_replace( citext, citext, text,
text ) returns TEXT AS '
SELECT regexp_replace( $1::text, $2::text, $3, CASE WHEN
strpos($4, ''c'') = 0 THEN $4 || ''i'' ELSE $4 END);
' LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION regexp_split_to_array( citext, citext )
RETURNS TEXT[] AS '
SELECT regexp_split_to_array( $1::text, $2::text, ''i'' );
' LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION regexp_split_to_array( citext, citext,
text ) RETURNS TEXT[] AS '
SELECT regexp_split_to_array( $1::text, $2::text, CASE WHEN
strpos($3, ''c'') = 0 THEN $3 || ''i'' ELSE $3 END );
' LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION regexp_split_to_table( citext, citext )
RETURNS SETOF TEXT AS '
SELECT regexp_split_to_table( $1::text, $2::text, ''i'' );
' LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION regexp_split_to_table( citext, citext,
text ) RETURNS SETOF TEXT AS '
SELECT regexp_split_to_table( $1::text, $2::text, CASE WHEN
strpos($3, ''c'') = 0 THEN $3 || ''i'' ELSE $3 END );
' LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION strpos( citext, citext ) RETURNS INT AS '
SELECT strpos( LOWER( $1::text ), LOWER( $2::text ) );
' LANGUAGE SQL IMMUTABLE STRICT;

Not so bad, though it'd be nice to have C functions that just did
these things. Still not case-insensitive are:

-- replace()
-- split_part()
-- translate()

So, anyone at OSCON this week want to help me with these? Or to
convert the above functions to C? Greg? Bruce?

Thanks,

David

On Jul 18, 2008, at 01:39, Michael Paesold wrote:

> Calling regex functions with the case-insensitivity option would be
> great. It should also be possible to rewrite replace() into
> regexp_replace() by first escaping the regex meta characters.
>
> Actually re-implementing those functions in a case insensitive way
> would still be an option, but of course some amount of work. The
> question is, how much use case there is.

I've figured out how to make all the functions work using SQL function
workarounds, converting things and re-dispatching to the text versions
as appropriate. They work quite well, and can be converted to C later
if that becomes a requirement.

Meanwhile, on the question of whether or not regular expression and
LIKE comparisons *should* match case-insensitively, I have a couple
more observations:

* Thinking about how a true case-insensitive collation would work, I'm
quite certain that it would match case-insensitively. Anything else
would just be unexpected, because in a case-insensitive collation,
lowercase characters are, in practice, identical to uppercase
characters. As far as matching is concerned, there is no difference
between them. So the matching operators and functions against CITEXT
should follow that assumption.

* I tried a few matches on MySQL, where the collation is case-
insensitive by default, and it confirms my impression:

mysql> select 'Foo' regexp 'o$';
+-------------------+
| 'Foo' regexp 'o$' |
+-------------------+
| 1 |
+-------------------+
1 row in set (0.00 sec)

mysql> select 'Foo' regexp 'O$';
+-------------------+
| 'Foo' regexp 'O$' |
+-------------------+
| 1 |
+-------------------+
1 row in set (0.00 sec)

mysql> select 'Foo' like '%o';
+-----------------+
| 'Foo' like '%o' |
+-----------------+
| 1 |
+-----------------+
1 row in set (0.00 sec)

mysql> select 'Foo' like '%O';
+-----------------+
| 'Foo' like '%O' |
+-----------------+
| 1 |
+-----------------+
1 row in set (0.00 sec)

I'll grant that MySQL may not be the best model for how things should
work, but it's something, at least. Anyone else got access to another
database with case-insensitive collations to see how LIKE and regular
expressions work?

Thanks,

David