I wrote:
> Works for me. I'm done for the day but will make it so tomorrow.
Pushed that, though in the event I wrote a rather longer apologia
for why we use ERROR in one complaint and WARNING in the other.
Getting back to the original bug ... I went through tuplestore.c
and found two other places where foreseeable failures in subroutines
could leave the tuplestore in an inconsistent state. I think we
need to fix them all, per the attached draft against HEAD. The
back branches might have more/different bugs, didn't look yet.
regards, tom lane
diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c
index caad7cad0b4..f9e2d95186a 100644
--- a/src/backend/utils/sort/tuplestore.c
+++ b/src/backend/utils/sort/tuplestore.c
@@ -708,10 +708,10 @@ grow_memtuples(Tuplestorestate *state)
/* OK, do it */
FREEMEM(state, GetMemoryChunkSpace(state->memtuples));
- state->memtupsize = newmemtupsize;
state->memtuples = (void **)
repalloc_huge(state->memtuples,
- state->memtupsize * sizeof(void *));
+ newmemtupsize * sizeof(void *));
+ state->memtupsize = newmemtupsize;
USEMEM(state, GetMemoryChunkSpace(state->memtuples));
if (LACKMEM(state))
elog(ERROR, "unexpected out-of-memory situation in tuplestore");
@@ -1306,7 +1306,19 @@ dumptuples(Tuplestorestate *state)
if (i >= state->memtupcount)
break;
WRITETUP(state, state->memtuples[i]);
+
+ /*
+ * Increase memtupdeleted to track the fact that we just deleted that
+ * tuple. Think not to remove this on the grounds that we'll reset
+ * memtupdeleted to zero below. We might not reach that if some later
+ * WRITETUP fails (e.g. due to overrunning temp_file_limit). If so,
+ * we'd error out leaving an effectively-corrupt tuplestore, which
+ * would be quite bad if it's a persistent data structure such as a
+ * Portal's holdStore.
+ */
+ state->memtupdeleted++;
}
+ /* Now we can reset memtupdeleted along with memtupcount */
state->memtupdeleted = 0;
state->memtupcount = 0;
}
@@ -1496,8 +1508,10 @@ tuplestore_trim(Tuplestorestate *state)
FREEMEM(state, GetMemoryChunkSpace(state->memtuples[i]));
pfree(state->memtuples[i]);
state->memtuples[i] = NULL;
+ /* As in dumptuples(), increment memtupdeleted synchronously */
+ state->memtupdeleted++;
}
- state->memtupdeleted = nremove;
+ Assert(state->memtupdeleted == nremove);
/* mark tuplestore as truncated (used for Assert crosschecks only) */
state->truncated = true;
