From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | "David E(dot) Wheeler" <david(at)kineticode(dot)com> |
Cc: | Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: PATCH: CITEXT 2.0 |
Date: | 2008-07-03 05:14:14 |
Message-ID: | 23892.1215062054@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
"David E. Wheeler" <david(at)kineticode(dot)com> writes:
> On Jul 2, 2008, at 09:13, Zdenek Kotala wrote:
>> Please, do not type space before asterix:
>> char * lcstr, * rcstr -> char *lcstr, *rcstr
>>
>> and do not put extra space in a brackets
>> citextcmp( left, right ) -> citextcmp(left, right)
> Okay.
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.
> Um, are you referring to this (at the top):
> +// 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.
> Yeah, I'm a bit confused by this. I followed what's in varlena.c on
> this. The comparison functions are documented supposed to return 1, 0,
> or -1, which of course would be covered by int. But varstr_cmp(),
> which ultimately returns the value, returns all kinds of numbers. So,
> perhaps someone could say what it's *supposed* to be?
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.
>> 3) citext_larger, citext_smaller function have memory leak. You
>> don't call PG_FREE_IF_COPY on unused text.
> These I also duplicated from varlena.c, and I asked about a potential
> memory leak in a previous email. If there's a leak here, would there
> not also be a leak in varlena.c?
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.
>> 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
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Ken Camann | 2008-07-03 05:45:06 | Re: A Windows x64 port of PostgreSQL |
Previous Message | Yoshiyuki Asaba | 2008-07-03 04:56:54 | Re: Git Repository for WITH RECURSIVE and others |