Re: [SQL] A bug in gistPageAddItem()/gist_tuple_replacekey() ??? (fwd) - Mailing list pgsql-hackers

From Teodor Sigaev
Subject Re: [SQL] A bug in gistPageAddItem()/gist_tuple_replacekey() ??? (fwd)
Date
Msg-id 3CBE7EEC.4010803@stack.net
Whole thread Raw
Responses Re: [SQL] A bug in gistPageAddItem()/gist_tuple_replacekey()  (Bruce Momjian <pgman@candle.pha.pa.us>)
List pgsql-hackers
gistPageAddItem and gist_tuple_replacekey are commented out by GIST_PAGEADDITEM. 
They was used up to 7.1, but now it is unusable.

gistPageAddItem has interesting feature: recompress entry before writing to 
disk, but we (with Oleg and Tom) couldn't find any reasons to do it. And so, we 
leave this code for later thinking about.

Now gistPageAddItem is wrong, because it can work only in single-key indexes. In 
7.2 GiST supports multy-key index.



> I haven't see any comment on this.  If no one replies, would you send
> over a patch of fixes?  Thanks.
> 
> ---------------------------------------------------------------------------
> 
> Dmitry Tkach wrote:
> 
>>I was trying to write a gist index extension, and, after some debugging,
>>it looks like I found a bug somewhere in the gist.c code ...
>>I can't be quite sure, because I am not familiar with the postgres
>>code... but, here is what I see happenning (this is 7.1, but I compared
>>the sources to 7.2, and did not see this fixed - although, I did not
>>inspect it too carefully)...
>>
>>First of all, gistPageAddItem () calls gistdentryinit() with a pointer
>>to what's stored in the tuple, so, 'by-value' types do not work (because
>>gistcentryinit () would be passed the value itself, when called from
>>gistinsert(), and then, in gistPageAddItem (), it is passed a pointer,
>>coming from gistdentryinit () - so, it just doesn't know really how to
>>treat the argument)...
>>
>>Secondly, gist_tuple_replacekey() seems to have incorrect logic figuring
>>out if there is enough space in the tuple (it checks for '<', instead of
>>'<=') - this causes a new tuple to get always created (this one, seems
>>to be fixed in 7.2)
>>
>>Thirdly, gist_tuple_replace_key () sends a pointer to entry.pred (which
>>is already a pointer to the actual value) to index_formtuple (), that
>>looks at the tuple, sees that the type is 'pass-by-value', and puts that
>>pointer directly into the tuple, so that, the resulting tuple now
>>contains a pointer to a pointer to the actual value...
>>
>>Now, if more then one split is required, this sequence is repeated again
>>and again and again, so that, by the time the tuple gets actually
>>written, it contains something like a pointer to a pointer to a pointer
>>to a pointer to the actual data :-(
>>
>>Once again, I've seen some comments in the 7.2 branch about gists and
>>pass-by-value types, but brief looking at the differences in the source
>>did not make me conveinced that it was indeed fixed...
>>
>>Anyone knows otherwise?
>>
>>Thanks a lot!
>>
>>Dima
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 4: Don't 'kill -9' the postmaster
>>
>>
> 


-- 
Teodor Sigaev
teodor@stack.net




pgsql-hackers by date:

Previous
From: Andreas Scherbaum
Date:
Subject: Re: new food for the contrib/ directory
Next
From: Maarten.Boekhold@reuters.com
Date:
Subject: Re: Index Scans become Seq Scans after VACUUM ANALYSE