diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c index 1f50993621..e7c681ab7f 100644 --- a/src/backend/commands/portalcmds.c +++ b/src/backend/commands/portalcmds.c @@ -389,26 +389,22 @@ PersistHoldablePortal(Portal portal) FreeQueryDesc(queryDesc); /* - * Set the position in the result set: ideally, this could be - * implemented by just skipping straight to the tuple # that we need - * to be at, but the tuplestore API doesn't support that. So we start - * at the beginning of the tuplestore and iterate through it until we - * reach where we need to be. FIXME someday? (Fortunately, the - * typical case is that we're supposed to be at or near the start of - * the result set, so this isn't as bad as it sounds.) + * Set the position in the result set. */ MemoryContextSwitchTo(portal->holdContext); if (portal->atEnd) { - /* we can handle this case even if posOverflow */ - while (tuplestore_advance(portal->holdStore, true)) + /* + * We can handle this case even if posOverflow: just force the + * tuplestore forward to its end. The size of the skip request + * here is arbitrary. + */ + while (tuplestore_skiptuples(portal->holdStore, 1000000, true)) /* continue */ ; } else { - long store_pos; - if (portal->posOverflow) /* oops, cannot trust portalPos */ ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), @@ -416,11 +412,10 @@ PersistHoldablePortal(Portal portal) tuplestore_rescan(portal->holdStore); - for (store_pos = 0; store_pos < portal->portalPos; store_pos++) - { - if (!tuplestore_advance(portal->holdStore, true)) - elog(ERROR, "unexpected end of tuple stream"); - } + if (!tuplestore_skiptuples(portal->holdStore, + portal->portalPos, + true)) + elog(ERROR, "unexpected end of tuple stream"); } } PG_CATCH(); diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 046637fb09..2fcc630a92 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -2371,31 +2371,56 @@ window_gettupleslot(WindowObject winobj, int64 pos, TupleTableSlot *slot) tuplestore_select_read_pointer(winstate->buffer, winobj->readptr); /* - * There's no API to refetch the tuple at the current position. We have to - * move one tuple forward, and then one backward. (We don't do it the - * other way because we might try to fetch the row before our mark, which - * isn't allowed.) XXX this case could stand to be optimized. + * Advance or rewind until we are within one tuple of the one we want. */ - if (winobj->seekpos == pos) + if (winobj->seekpos < pos - 1) { + if (!tuplestore_skiptuples(winstate->buffer, + pos - 1 - winobj->seekpos, + true)) + elog(ERROR, "unexpected end of tuplestore"); + winobj->seekpos = pos - 1; + } + else if (winobj->seekpos > pos + 1) + { + if (!tuplestore_skiptuples(winstate->buffer, + winobj->seekpos - (pos + 1), + false)) + elog(ERROR, "unexpected end of tuplestore"); + winobj->seekpos = pos + 1; + } + else if (winobj->seekpos == pos) + { + /* + * There's no API to refetch the tuple at the current position. We + * have to move one tuple forward, and then one backward. (We don't + * do it the other way because we might try to fetch the row before + * our mark, which isn't allowed.) XXX this case could stand to be + * optimized. + */ tuplestore_advance(winstate->buffer, true); winobj->seekpos++; } - while (winobj->seekpos > pos) + /* + * Now we should be on the tuple immediately before or after the one we + * want, so just fetch forwards or backwards as appropriate. + */ + if (winobj->seekpos > pos) { if (!tuplestore_gettupleslot(winstate->buffer, false, true, slot)) elog(ERROR, "unexpected end of tuplestore"); winobj->seekpos--; } - - while (winobj->seekpos < pos) + else { if (!tuplestore_gettupleslot(winstate->buffer, true, true, slot)) elog(ERROR, "unexpected end of tuplestore"); winobj->seekpos++; } + Assert(winobj->seekpos == pos); + MemoryContextSwitchTo(oldcontext); return true; @@ -2478,16 +2503,20 @@ WinSetMarkPosition(WindowObject winobj, int64 markpos) if (markpos < winobj->markpos) elog(ERROR, "cannot move WindowObject's mark position backward"); tuplestore_select_read_pointer(winstate->buffer, winobj->markptr); - while (markpos > winobj->markpos) + if (markpos > winobj->markpos) { - tuplestore_advance(winstate->buffer, true); - winobj->markpos++; + tuplestore_skiptuples(winstate->buffer, + markpos - winobj->markpos, + true); + winobj->markpos = markpos; } tuplestore_select_read_pointer(winstate->buffer, winobj->readptr); - while (markpos > winobj->seekpos) + if (markpos > winobj->seekpos) { - tuplestore_advance(winstate->buffer, true); - winobj->seekpos++; + tuplestore_skiptuples(winstate->buffer, + markpos - winobj->seekpos, + true); + winobj->seekpos = markpos; } } diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c index bcba30e598..a5a56be91b 100644 --- a/src/backend/utils/sort/tuplestore.c +++ b/src/backend/utils/sort/tuplestore.c @@ -57,6 +57,7 @@ #include "access/htup_details.h" #include "commands/tablespace.h" #include "executor/executor.h" +#include "miscadmin.h" #include "storage/buffile.h" #include "utils/memutils.h" #include "utils/resowner.h" @@ -1065,8 +1066,7 @@ tuplestore_gettupleslot(Tuplestorestate *state, bool forward, * tuplestore_advance - exported function to adjust position without fetching * * We could optimize this case to avoid palloc/pfree overhead, but for the - * moment it doesn't seem worthwhile. (XXX this probably needs to be - * reconsidered given the needs of window functions.) + * moment it doesn't seem worthwhile. */ bool tuplestore_advance(Tuplestorestate *state, bool forward) @@ -1088,6 +1088,75 @@ tuplestore_advance(Tuplestorestate *state, bool forward) } } +/* + * Advance over N tuples in either forward or back direction, + * without returning any data. N<=0 is a no-op. + * Returns TRUE if successful, FALSE if ran out of tuples. + */ +bool +tuplestore_skiptuples(Tuplestorestate *state, int64 ntuples, bool forward) +{ + TSReadPointer *readptr = &state->readptrs[state->activeptr]; + + Assert(forward || (readptr->eflags & EXEC_FLAG_BACKWARD)); + + if (ntuples <= 0) + return true; + + switch (state->status) + { + case TSS_INMEM: + if (forward) + { + if (readptr->eof_reached) + return false; + if (state->memtupcount - readptr->current >= ntuples) + { + readptr->current += ntuples; + return true; + } + readptr->current = state->memtupcount; + readptr->eof_reached = true; + return false; + } + else + { + if (readptr->eof_reached) + { + readptr->current = state->memtupcount; + readptr->eof_reached = false; + ntuples--; + } + if (readptr->current - state->memtupdeleted > ntuples) + { + readptr->current -= ntuples; + return true; + } + Assert(!state->truncated); + readptr->current = state->memtupdeleted; + return false; + } + break; + + default: + /* We don't currently try hard to optimize other cases */ + while (ntuples-- > 0) + { + void *tuple; + bool should_free; + + tuple = tuplestore_gettuple(state, forward, &should_free); + + if (tuple == NULL) + return false; + if (should_free) + pfree(tuple); + CHECK_FOR_INTERRUPTS(); + } + return true; + } +} + /* * dumptuples - remove tuples from memory and write to tape * diff --git a/src/include/utils/tuplestore.h b/src/include/utils/tuplestore.h index 1e22feecee..16eca871cd 100644 --- a/src/include/utils/tuplestore.h +++ b/src/include/utils/tuplestore.h @@ -72,8 +72,12 @@ extern bool tuplestore_in_memory(Tuplestorestate *state); extern bool tuplestore_gettupleslot(Tuplestorestate *state, bool forward, bool copy, TupleTableSlot *slot); + extern bool tuplestore_advance(Tuplestorestate *state, bool forward); +extern bool tuplestore_skiptuples(Tuplestorestate *state, + int64 ntuples, bool forward); + extern bool tuplestore_ateof(Tuplestorestate *state); extern void tuplestore_rescan(Tuplestorestate *state);