Re: PATCH: CITEXT 2.0

On Jul 2, 2008, at 22:14, Tom Lane wrote:

> Note that this sort of stuff will mostly be fixed by pg_indent,
> whether or not David does anything about it.  But certainly
> conforming to the project style to begin with will cause less
> pain to reviewers' eyeballs.

Yeah, I'll change it. I'm JAPH, so kind of made up the formatting as I  
went, though I did try to copy the style in varlena.c.

>> +// PostgreSQL 8.2 Magic.
>> +#ifdef PG_MODULE_MAGIC
>> +PG_MODULE_MAGIC;
>> +#endif
>
> Here however is an outright bug: we do not allow // comments,  
> because we
> still support ANSI-spec compilers that don't recognize them.

Forgot about that. I'll change it for the next version.

> btree cmp functions are allowed to return int32 negative, zero, or
> positive.  There is *not* any requirement that negative or positive
> results be exactly -1 or +1.  However, if you are comparing values
> that are int32 or wider then you can't just return their difference
> because it might overflow.

Thanks for the explanation. I'll make sure that they're both int32.

> The "leak" is irrelevant for larger/smaller.  The only place where  
> it's
> actually useful to do PG_FREE_IF_COPY is in a btree or hash index
> support function.  In other cases you can assume that you're being
> called in a memory context that's too short-lived for it to matter.

So would that be for any function used by

CREATE OPERATOR CLASS citext_ops
DEFAULT FOR TYPE CITEXT USING btree AS    OPERATOR    1   <  (citext, citext),    OPERATOR    2   <= (citext, citext),
 OPERATOR    3   =  (citext, citext),    OPERATOR    4   >= (citext, citext),    OPERATOR    5   >  (citext, citext),
FUNCTION    1   citext_cmp(citext, citext);
 

? (And then the btree operator and function to be added, of course.)

>
>
>>> 5) There are several commented out lines in CREATE OPERATOR
>>> statement mostly related to NEGATOR. Is there some reason for that?
>
>> I copied it from the original citext.sql. Not sure what effect it  
>> has.
>
> http://developer.postgresql.org/pgdocs/postgres/xoper- 
> optimization.html

Thanks. Sounds like it'd be valuable to have them in there. I'll add  
tests, as well.

Best,

David