From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <dgrowleyml@gmail.com>
cc: kuzmin.db4@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #19438: segfault with temp_file_limit inside cursor
In-reply-to: <1897027.1774837151@sss.pgh.pa.us>
References: <19438-9d37b179c56d43aa@postgresql.org> <1106026.1774573371@sss.pgh.pa.us> <1338824.1774633289@sss.pgh.pa.us> <CAApHDvox3Ro8mZJxignuyB-dGXJ9=wQNEkOFni9025GP=rOKkg@mail.gmail.com> <1830345.1774798374@sss.pgh.pa.us> <CAApHDvrdPriH6MO=YGEh9=KVDnDtqZyW4VuRXkmBS8WnRFessA@mail.gmail.com> <1881853.1774828272@sss.pgh.pa.us> <CAApHDvpZOCWwSNuBZ4Xg-EcbQ9Bxbk+58AsuE1FCbV5=wyXiqw@mail.gmail.com> <1886754.1774830888@sss.pgh.pa.us> <CAApHDvqv7g3QODYWbaokXrB9eZrY6JkOVO8cO_TXu_PiU_vyOg@mail.gmail.com> <1897027.1774837151@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Sun, 29 Mar 2026 22:19:11 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <2333378.1774888128.0@sss.pgh.pa.us>
Date: Mon, 30 Mar 2026 12:29:20 -0400
Message-ID: <2333427.1774888160@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/2333427.1774888160%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2333378.1774888128.1@sss.pgh.pa.us>

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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="v2-fix-tuplestore-inconsistency-after-error.patch";
	charset="us-ascii"
Content-ID: <2333378.1774888128.2@sss.pgh.pa.us>
Content-Description: v2-fix-tuplestore-inconsistency-after-error.patch
Content-Transfer-Encoding: quoted-printable

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 =3D newmemtupsize;
 	state->memtuples =3D (void **)
 		repalloc_huge(state->memtuples,
-					  state->memtupsize * sizeof(void *));
+					  newmemtupsize * sizeof(void *));
+	state->memtupsize =3D 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 >=3D 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 =3D 0;
 	state->memtupcount =3D 0;
 }
@@ -1496,8 +1508,10 @@ tuplestore_trim(Tuplestorestate *state)
 		FREEMEM(state, GetMemoryChunkSpace(state->memtuples[i]));
 		pfree(state->memtuples[i]);
 		state->memtuples[i] =3D NULL;
+		/* As in dumptuples(), increment memtupdeleted synchronously */
+		state->memtupdeleted++;
 	}
-	state->memtupdeleted =3D nremove;
+	Assert(state->memtupdeleted =3D=3D nremove);
 =

 	/* mark tuplestore as truncated (used for Assert crosschecks only) */
 	state->truncated =3D true;

------- =_aaaaaaaaaa0--