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


Thu, 21 Mar 2002 16:33:31 -0500
Dmitry Tkach <dmitry@openratings.com>
A bug in gistPageAddItem()/gist_tuple_replacekey() ???

[ Cc to hackers.]

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
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Wed, 17 Apr 2002 17:58:31 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [SQL] A bug in gistPageAddItem()/gist_tuple_replacekey() ???