James Coleman <jtc...@gmail.com> writes: > While digging into the incremental sort patch, I noticed in > tuplesort.c at the beginning of the function in $SUBJECT we have this > comment and assertion:
> tuplesort_set_bound(Tuplesortstate *state, int64 bound) > { > /* Assert we're called before loading any tuples */ > Assert(state->status == TSS_INITIAL); > But AFAICT from reading the code in puttuple_common the state remains > TSS_INITIAL while tuples are inserted (unless we reach a point where > we decide to transition it to TSS_BOUNDED or TSS_BUILDRUNS). You missed the relevance of the next line: Assert(state->memtupcount == 0); I think the comment is fine as-is. Perhaps the code would be clearer though, if we merged those two asserts into one? /* Assert we're called before loading any tuples */ Assert(state->status == TSS_INITIAL && state->memtupcount == 0); I'm not totally sure about the usefulness/relevance of the two assertions following these, but they could likely do with their own comment(s), because this one surely isn't covering them. regards, tom lane