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 bit. > 2) It would be better if childoffnum have the same semantics in regular > insert and fastbuild insert.
Hi Alexander, I've looked at your patch and have done a "partial" review. It applies cleanly and makes without warnings, and passes make check plus some additional testing I've done (inserting lots of stuff into regression's test_tsvector table, in parallel, with the gist index in place) under --enable-cassert. I repeated that test without --enable-cassert, and saw no degradation in performance over unpatched code. No changes to documentation or "make check" code should be needed. The formatting looks OK. 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 to myself from first principles. I have no reason to believe it is not correct, it is just that I can't convince myself that it is correct. 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 obscure situation which I can't induce at will. I would love to use this as an opportunity to study the gist code until I can convince myself this patch is correct, but I'm afraid I won't be able to do that promptly, or for the remainder of this commit fest. Since Heikki has already looked at this patch, perhaps he can provide the assurance that I cannot, or another reviewer can. Sorry I couldn't do a more thorough review, Jeff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
