On Tue, 2012-09-04 at 19:21 +0400, Alexander Korotkov wrote:
> New version of patch is attached. Parameter "randomization" was
> introduced. It controls whether to randomize choose. Choose algorithm
> was rewritten.
>
Review comments:
1. Comment above while loop in gistRelocateBuildBuffersOnSplit needs to
be updated.
2. Typo in two places: "if randomization id required".
3. In gistRelocateBuildBuffersOnSplit, shouldn't that be: splitPageInfo = &relocationBuffersInfos[bufferIndex];
not: splitPageInfo = &relocationBuffersInfos[i];
4. It looks like the randomization is happening while trying to compare
the penalties. I think it may be more readable to separate those two
steps; e.g.
/* create a mapping whether randomization is on or not */ for (i = FirstOffsetNumber; i <= maxoff; i =
OffsetNumberNext(i)) offsets[i - FirstOffsetNumber] = i;
if (randomization) /* randomize offsets array */
for (i = 0; i < maxoff; i++) { offset = offsets[i]; ... }
That's just an idea; if you think it's more readable as-is (or if I am
misunderstanding) then let me know.
Regards,Jeff Davis
