From: | James Coleman <jtc331(at)gmail(dot)com> |
---|---|
To: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Misleading comment in tuplesort_set_bound |
Date: | 2019-06-23 19:22:04 |
Message-ID: | CAAaqYe9GD__4Crm=ddz+-XXcNhfY_V5gFYdLdmkFNq=2VHO56Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
tuplesort_set_bound_comment-v1.patch | text/x-patch | 1.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2019-06-23 19:44:15 | Re: how to run encoding-dependent tests by default |
Previous Message | Peter Eisentraut | 2019-06-23 18:13:28 | Re: Add CREATE DATABASE LOCALE option |