On Tue, Nov 14, 2017 at 1:41 AM, Rushabh Lathia <rushabh.lat...@gmail.com> wrote: > Thanks Peter and Thomas for the review comments.
No problem. More feedback: * I don't really see much need for this: + elog(LOG, "Worker for create index %d", parallel_workers); You can just use trace_sort, and observe the actual behavior of the sort that way. * As I said before, you should remove the header comments within nbtsort.c. * This should just say "write routines": + * This is why write/recycle routines don't need to know about offsets at + * all. * You didn't point out the randomAccess restriction in tuplesort.h. * I can't remember why I added the Valgrind suppression at this point. I'd remove it until the reason becomes clear, which may never happen. The regression tests should still pass without Valgrind warnings. * You can add back comments removed from above LogicalTapeTell(). I made these changes because it looked like we should close out the possibility of doing a tell during the write phase, as unified tapes actually would make that hard (no one does what it describes anyway). But now, unified tapes are a distinct case to frozen tapes in a way that they weren't before, so there is no need to make it impossible. I also think you should replace "Assert(lt->frozen)" with "Assert(lt->offsetBlockNumber == 0L)", for the same reason. -- Peter Geoghegan
