Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-15 Thread Heikki Linnakangas
On 14.07.2011 13:29, Alexander Korotkov wrote: On Thu, Jul 14, 2011 at 12:56 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: First, notice that we're setting ptr-parent = top. 'top' is the current node we're processing, and ptr represents the node to the right of the current

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-15 Thread Heikki Linnakangas
On 13.07.2011 22:04, Heikki Linnakangas wrote: On 13.07.2011 21:56, Alexander Korotkov wrote: Thank you very much for detail explanation. But this line of modified patch seems strange for me: *newchildoffnum = blkno; I believe it should be: *newchildoffnum = i; Yes, you're right. It's scary

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-15 Thread Alexander Korotkov
On Fri, Jul 15, 2011 at 1:27 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: Ok, committed this now. Thank you. I decided to rename the childoffnum field to downlinkoffnum. I figured it'd be dangerous that the field means something subtly different in different versions,

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-14 Thread Alexander Korotkov
On Thu, Jul 14, 2011 at 12:56 PM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: First, notice that we're setting ptr-parent = top. 'top' is the current node we're processing, and ptr represents the node to the right of the current node. The current node is *not* the parent of

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-14 Thread Heikki Linnakangas
I think there's two bugs in the existing gistFindPath code: if (top-parent XLByteLT(top-parent-lsn, GistPageGetOpaque(page)-nsn) GistPageGetOpaque(page)-rightlink != InvalidBlockNumber /* sanity check */ ) { /*

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-13 Thread Heikki Linnakangas
On 10.07.2011 21:43, Josh Berkus wrote: Teodor, Oleg, Heikki, My concern is that I am unable to prove to myself simply by reading the code that the 24 line chunk deleted from gistFindPath (near *** 919,947 ) are no longer needed. My familiarity with the gist code is low enough that it is

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-13 Thread Heikki Linnakangas
On 30.06.2011 07:50, Jeff Janes wrote: My concern is that I am unable to prove to myself simply by reading the code that the 24 line chunk deleted from gistFindPath (near *** 919,947 ) are no longer needed. My familiarity with the gist code is low enough that it is not surprising that I

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-13 Thread Alexander Korotkov
Thank you very much for detail explanation. But this line of modified patch seems strange for me: *newchildoffnum = blkno; I believe it should be: *newchildoffnum = i; -- With best regards, Alexander Korotkov.

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-13 Thread Heikki Linnakangas
On 13.07.2011 21:56, Alexander Korotkov wrote: Thank you very much for detail explanation. But this line of modified patch seems strange for me: *newchildoffnum = blkno; I believe it should be: *newchildoffnum = i; Yes, you're right. It's scary that it worked during testing anyway. Maybe the

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-07-10 Thread Josh Berkus
Teodor, Oleg, Heikki, My concern is that I am unable to prove to myself simply by reading the code that the 24 line chunk deleted from gistFindPath (near *** 919,947 ) are no longer needed. My familiarity with the gist code is low enough that it is not surprising that I cannot prove this

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-06-30 Thread Alexander Korotkov
Hi Jeff, Thank you for review. On Thu, Jun 30, 2011 at 8:50 AM, Jeff Janes jeff.ja...@gmail.com wrote: So I tried provoking situations where this surrounding code section would get executed, both patched and unpatched. I have been unable to do so--apparently this code is for an incredibly

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-06-29 Thread Jeff Janes
On Tue, Jun 28, 2011 at 10:21 AM, Alexander Korotkov aekorot...@gmail.com wrote: Actually, there is no more direct need of this patch because I've rewrote insert function for fast build. But there are still two points for having this changes: 1) As it was noted before, it simplifies code a

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-06-28 Thread Alexander Korotkov
Actually, there is no more direct need of this patch because I've rewrote insert function for fast build. But there are still two points for having this changes: 1) As it was noted before, it simplifies code a bit. 2) It would be better if childoffnum have the same semantics in regular insert and

Re: [HACKERS] Small patch for GiST: move childoffnum to child

2011-05-30 Thread Heikki Linnakangas
On 24.05.2011 15:22, Alexander Korotkov wrote: During preparing patch of my GSoC project I found reasonable to move childoffnum (GISTInsertStack structure) from parent to child. This means that now child have childoffnum of parent's link to child. It allows to maintain entire parts of tree in