Re: PATCH: CITEXT 2.0 v2

David E. Wheeler napsal(a):
> On Jun 27, 2008, at 18:22, David E. Wheeler wrote:
>
>> Please find attached a patch adding a locale-aware, case-insensitive
>> text type, called citext, as a contrib module.
>
> Here is a new version of the patch, with the following changes:
>
> * Fixed formatting to be more like core.
> * Added appropriate NEGATORs to operators.
> * Removed NEGATOR from the || operator.
> * Added hash index function and operator class.
> * The = operator now supports HASHES and MERGES.
> * citext_cmp and citextcmp both return int32.
> * Changed // comments to /* comments */.
> * Added test confirming láska'::citext <> 'laská'::citext.
> * A few other organizational, formatting, and pasto fixes.
> * Updated the FAQ entry on case-insensitive queries to recommend citext
> (it would, of course, need to be translated).
>
> Stuff I was asked about but didn't change:
>
> * citext_cmp() still uses varstr_cmp() instead of strncmp(). When I
> tried the latter, everything seemed to be equivalent.

citext_eq (operator =) should use strncmp and citext_cmp() should use varstr_cmp().

> * citext_smaller() and citext_larger() don't have memory leaks, says
> Tom, so I added no calls to PG_FREE_IF_COPY().

Yeah, it is correct. FGMR does clean job for you.

However, It seems to me that code is ok now (exclude citex_eq). I think there
two open problems/questions:

1) regression test -

a) I think that regresion is not correct. It depends on en_US locale, but it
uses characters which is not in related character repertoire. It means comparing
is not defined and I guess it could generate different result on different OS -
depends on locale implementation.

b) pgTap is something new. Need make a decision if this framework is
acceptable or not.

2) contrib vs. pgFoundry

There is unresolved answer if we want to have this in contrib or not. Good to
mention that citext type will be obsoleted with full collation implementation in
a future. I personally prefer to keep it on pgFoundry because it is temporally
workaround (by my opinion), but I can live with contrib module as well.

Zdenek