On Mon, Nov 15, 2010 at 4:06 AM, Alexander Korotkov <aekorot...@gmail.com> wrote: > With help of Oleg I found, that line "*left = *right = FirstOffsetNumber;" > was needed only for 7.X compatibility, and it isn't needed any more. > Also, I've replaced "i - 1" by "i - FirstOffsetNumber" in array filling. > I believe it's more correct way, because it'll work correctly in the case > when FirstOffsetNumber alters.
The loop that begins here: for (i = 0; i < maxoff; i++) { /* First half of segs goes to the left datum. */ if (i < seed_2) ...looks like it should perhaps be broken into two separate loops. That might also help tweak the logic in a way that eliminates this: seg.c: In function ‘gseg_picksplit’: seg.c:327: warning: ‘datum_r’ may be used uninitialized in this function seg.c:326: warning: ‘datum_l’ may be used uninitialized in this function But on a broader note, I'm not very certain the sorting algorithm is sensible. For example, suppose you have 10 segments that are exactly '0' and 20 segments that are exactly '1'. Maybe I'm misunderstanding, but it seems like this will result in a 15/15 split when we almost certainly want a 10/20 split. I think there will be problems in more complex cases as well. The documentation says about the less-than and greater-than operators that "These operators do not make a lot of sense for any practical purpose but sorting." -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers