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).
Therefore it's not true that the assertion guards against having
loaded any tuples; rather it guarantees that we remain in standard
memory quicksort mode.
Assuming my understanding is correct, I've attached a small patch to
update the comment to "Assert we're still in memory quicksort mode and
haven't transitioned to heap or tape mode".
Note: this also means the function header comment "Must be called
before inserting any tuples" is a condition that isn't actually
validated, but I think that's fine given it's not a new problem and
even more so since the same comment goes on to say that that's
probably not a strict requirement.
James Coleman
commit bd185bbac4275299a587eae67eae58f856052a93
Author: jcoleman <james.cole...@getbraintree.com>
Date: Sun Jun 23 19:13:09 2019 +0000
Correct assertion comment in tuplesort_set_bound
The implementation of puttuple_common keeps the state TSS_INITIAL
while tuples are inserted unless we reach a point where we decide
to transition to a different mode and set the state to TSS_BOUNDED
or TSS_BUILDRUNS.
Therefore it's not true that the assertion guards against having loaded
any tuples; rather it guarantees that we remain in standard memory
quicksort mode.
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 7b8e67899e..c108bd4513 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -1185,7 +1185,9 @@ tuplesort_begin_datum(Oid datumType, Oid sortOperator, Oid sortCollation,
void
tuplesort_set_bound(Tuplesortstate *state, int64 bound)
{
- /* Assert we're called before loading any tuples */
+ /*
+ * Assert we're still in memory quicksort mode and haven't transitioned to
+ * heap or tape mode. */
Assert(state->status == TSS_INITIAL);
Assert(state->memtupcount == 0);
Assert(!state->bounded);
