On Mon, Jan 23, 2017 at 1:55 AM, Andrew Borodin <boro...@octonica.com> wrote: > Proposed patch, on a contrary, simplifies code. There is more code > deleted than added. It does not affect WAL, changes nothing in page > layout.
I appreciate that it is fewer lines of code. My hesitation comes from a couple areas: 1. I haven't seen the approach before of finding the parent of any empty subtree. While that sounds like a reasonable thing to do, it worries me to invent new approaches in an area as well-studied as btrees. The problem is less that it's too complex, and more that it's different. Future developers looking at this code will expect it to be either very simple or very standard, because it's just a posting list. 2. It relies on searches to only start from the very leftmost page (correct?). If someone wants to optimize the intersection of two posting trees later, they might break it if they don't understand the assumptions in this patch. 3. This adds a heap allocation, and copies the contents of the page, and then releases the pin and lock. GinStepRight is careful to avoid doing that (it locks the target before releasing the page containing the pointer). I don't see a problem in your patch, but again, we are breaking an assumption that future developers might make. Your patch solves a real problem (a 90-second stall is clearly not good) and I don't want to dismiss that. But I'd like to consider some alternatives that may not have these downsides. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers