"Alvaro Herrera" <[EMAIL PROTECTED]> writes:

> Gregory Stark wrote:
>> Attached is a small patch which fixes this case. It also makes the check
>> slightly more liberal -- we don't need to resort if the previous sort was
>> unbounded or the bound was greater than or equal to the new bound.
> Huh, can you clarify this comment:
> +        * XXX It would be nice to check tuplesortstate->boundUsed too but 
> that
> +        * seems like an abstraction violation. And for that matter to check
> +        * the tuplesort to see if randomaccess is possible even if it wasn't
> +        * requested so we don't resort input when the parameters haven't
> +        * changed if it was sorted in memory.
> I'm having serious trouble parsing it.

Well in the executor currently we check node->boundedDone and node->boundDone
to see whether the previous execution of this node had a bound. If it did and
we now don't or if it did but now our bound is larger then we have to

However the tuplesort may have actually done an unbounded sort either because
the bound (actually bound*2) may not have been reached or because it spilled
to disk and we did a disk sort. It would be nice to check that and not have to
reexecute unnecessarily.

But that means peeking into tuplesort's state. I started doing a patch
yesterday to do this by exporting a new method on tuplesort:

tuplesort_random_access(Tuplesortstate *state, bool bounded, unsigned 
        switch (state->status)
                case TSS_FINALMERGE:
                        return false;
                case TSS_SORTEDONTAPE:
                        return state->randomAccess;
                case TSS_SORTEDINMEM:
                        return (!state->boundUsed || (bounded && state->bound > 
                        return false;

That solves the abstraction barrier issue but I'm not sure if it's quite
detailed enough. Really Sort only needs to rewind the tuplesort which it can
do even if state->randomAccess is false. We may need two separate functions,
one which tests if rewind is supported and another which tests if random
access is supported. Also, I haven't looked at how the final merge case is
handled. It might be possible to support rescan (but not random access) in
that state as well.

  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Reply via email to