Re: PATCH: CITEXT 2.0 v3

From: "David E(dot) Wheeler" <david(at)kineticode(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: PATCH: CITEXT 2.0 v3
Date: 2008-07-11 21:31:58
Message-ID: 27083F56-6EEA-47BB-ABA5-E905D27AC713@kineticode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Jul 11, 2008, at 13:02, Zdenek Kotala wrote:

>> Thank you, Zdenek. Have you had a chance to try citext yet? Or did
>> you just read the source?
>
> I tested version two on Solaris/SPARC and Sun studio compiler. I
> checked last version only quickly (comparing your changes).

Thanks.

I just updated my performance test script (attached) by increasing the
number of rows tested by an order of magnitude. So now it creates
1,000,000 rows, queries them, adds indexes, and then queries them
again. Unfortunately, CITEXT seems to have a memory leak somewhere,
because when I index the CITEXT column, it fails with "ERROR: out of
memory". So obviously something's not getting cleaned up. Here's the
btree indexing function:

Datum
citext_cmp(PG_FUNCTION_ARGS)
{
text *left = PG_GETARG_TEXT_PP(0);
text *right = PG_GETARG_TEXT_PP(1);
int32 result;

result = citextcmp(left, right);

PG_FREE_IF_COPY(left, 0);
PG_FREE_IF_COPY(right, 1);

PG_RETURN_INT32(result);
}

And here's citextcmp():

citextcmp (text *left, text *right)
{
char *lcstr, *rcstr;
int result;

lcstr = str_tolower(VARDATA_ANY(left), VARSIZE_ANY_EXHDR(left));
rcstr = str_tolower(VARDATA_ANY(right), VARSIZE_ANY_EXHDR(right));

result = varstr_cmp(
lcstr,
VARSIZE_ANY_EXHDR(left),
rcstr,
VARSIZE_ANY_EXHDR(right)
);

pfree(lcstr);
pfree(rcstr);
return result;
}

Can anyone see where I'm failing to free up memory? Might it be in
some other function?

Thanks!

David

Attachment Content-Type Size
try.sql application/octet-stream 2.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Teodor Sigaev 2008-07-11 22:58:54 Re: [PATCHES] GIN improvements
Previous Message Tom Lane 2008-07-11 21:26:15 Re: Vacuuming leaked temp tables (once again)