On 2010-11-10 14:27, Alvaro Herrera wrote:
Hmm, the second for loop in gseg_picksplit uses "i< maxoff" whereas the
other one uses<=. The first is probably correct; if the second is also
correct it merits a comment on the discrepancy (To be honest, I'd get
rid of the "-1" in computing maxoff and use< in both places, given that
offsets are 1-indexed).
Good point. The second loop walks over the sorted array, which is
0-indexed. Do you favor making the sort array 1-indexed, like done in
e.g. gbt_num_picksplit? The downside is that the sort array is
initialized with length maxoff + 1: puzzling on its own, even more since
maxoff itself is initialized as entryvec->n -1. Note also that the qsort
call for the 1-indexed variant is more complex since it must skip the
first element.
probably should be OffsetNumberNext just to stay consistent with the
rest of the code.
Yes.
The assignment to *left and *right at the end of the routine seem pretty
useless (not to mention the comment talking about a routine that doesn't
exist anywhere).
They are necessary and it is code untouched by this patch, and the same
line occurs in other picksplit functions as well. The gbt_num_picksplit
function shows that it can be avoided, by rewriting in the second loop
*left++ = sortItems[i].index;
into
v->spl_left[v->spl_nleft] = sortItems[i].index
Even though this is longer code, I prefer this variant over the shorter one.
regards,
Yeb Havinga
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
