makesign() broken in tsearch2

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Teodor Sigaev <teodor(at)sigaev(dot)ru>, Oleg Bartunov <oleg(at)sai(dot)msu(dot)su>
Cc: pgsql-hackers(at)postgreSQL(dot)org
Subject: makesign() broken in tsearch2
Date: 2006-01-20 22:03:34
Message-ID: 27381.1137794614@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I noticed the following code in tsearch2:

typedef uint64 TPQTGist;

static TPQTGist
makesign(QUERYTYPE * a)
{
int i;
ITEM *ptr = GETQUERY(a);
TPQTGist sign = 0;

for (i = 0; i < a->size; i++)
{
if (ptr->type == VAL)
sign |= 1 << (ptr->val % SIGLEN);
ptr++;
}

return sign;
}

This is wrong because "1" is an int constant, not an int64, and
therefore the shift will be done in int width. Correct would be

sign |= ((TPQTGist) 1) << (ptr->val % SIGLEN);

The effect of this is at least that the high-order half of sign
remains always zero. Depending on what the machine does with shifts
exceeding the word width (which is undefined according to ANSI C),
the bottom half might be messed up too. So we are failing to exploit
the full intended "sign" space, which presumably is costing something
in index efficiency.

It looks to me like the values calculated by this routine end up on
disk, and therefore we can't fix it without forcing an initdb, or
at least REINDEX of all affected indexes. Is that correct?

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2006-01-20 22:58:33 Re: panic on 7.3
Previous Message Mike Rylander 2006-01-20 21:34:20 Re: Surrogate keys (Was: enums)