Fix handling of sortKeys field in Tuplesortstate.

robertmhaas · robertmhaas · commit 2720e96a9bd5 · 2015-03-09T10:35:41.000-04:00
Commit 5cefbf5 introduced an assumption that this field would always be non-NULL when doing a merge pass, but that's not true. Without this fix, you can crash the server by building a hash index that is sufficiently large relative to maintenance_work_mem, or by triggering a large datum sort. Commit 5ea86e6 changed the comments for that field to say that it would be set in all cases except for the hash index case, but that wasn't (and still isn't) true. The datum-sort failure was spotted by Tomas Vondra; initial analysis of that failure was by Peter Geoghegan. The remaining issues were spotted by me during review of the surrounding code, and the patch is all my fault.
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
@@ -336,9 +336,9 @@ struct Tuplesortstate
 	bool		markpos_eof;	/* saved "eof_reached" */
 
 	/*
-	 * The sortKeys variable is used by every case other than the hash index
-	 * case; it is set by tuplesort_begin_xxx.  tupDesc is only used by the
-	 * MinimalTuple and CLUSTER routines, though.
+	 * The sortKeys variable is used by every case other than the datum and
+	 * hash index cases; it is set by tuplesort_begin_xxx.  tupDesc is only
+	 * used by the MinimalTuple and CLUSTER routines, though.
 	 */
 	TupleDesc	tupDesc;
 	SortSupport sortKeys;		/* array of length nKeys */
@@ -1246,7 +1246,7 @@ tuplesort_putindextuplevalues(Tuplesortstate *state, Relation rel,
 							 RelationGetDescr(state->indexRel),
 							 &stup.isnull1);
 
-	if (!state->sortKeys->abbrev_converter || stup.isnull1)
+	if (!state->sortKeys || !state->sortKeys->abbrev_converter || stup.isnull1)
 	{
 		/*
 		 * Store ordinary Datum representation, or NULL value.  If there is a
@@ -2172,7 +2172,7 @@ mergeruns(Tuplesortstate *state)
 		return;
 	}
 
-	if (state->sortKeys->abbrev_converter)
+	if (state->sortKeys != NULL && state->sortKeys->abbrev_converter != NULL)
 	{
 		/*
 		 * If there are multiple runs to be merged, when we go to read back
