Re: PATCH: CITEXT 2.0 v2 - Mailing list pgsql-hackers

From Zdenek Kotala
Subject Re: PATCH: CITEXT 2.0 v2
Date
Msg-id 48722B37.4020901@sun.com
Whole thread Raw
In response to PATCH: CITEXT 2.0 v2  (David E. Wheeler <david@kineticode.com>)
Responses Re: PATCH: CITEXT 2.0 v2  (Andrew Dunstan <andrew@dunslane.net>)
Re: PATCH: CITEXT 2.0 v2  ("David E. Wheeler" <david@kineticode.com>)
List pgsql-hackers
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




pgsql-hackers by date:

Previous
From: David Fetter
Date:
Subject: Re: [PATCHES] WITH RECURSIVE updated to CVS TIP
Next
From: Tom Lane
Date:
Subject: Re: [PATCHES] WIP: executor_hook for pg_stat_statements