Re: [SQL] A bug in gistPageAddItem()/gist_tuple_replacekey() ??? (fwd)

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(at)stack(dot)net