Fix GiST index-only scans for opclasses with different storage type.

hlinnaka · hlinnaka · commit 55b59eda13a7 · 2015-03-26T23:07:52.000+02:00
We cannot use the index's tuple descriptor directly to describe the index
tuples returned in an index-only scan. That's because the index might use
a different datatype for the values stored on disk than the type originally
indexed. As long as they were both pass-by-ref, it worked, but will not work
for pass-by-value types of different sizes. I noticed this as a crash when I
started hacking a patch to add fetch methods to btree_gist.
diff --git a/src/backend/access/gist/gistscan.c b/src/backend/access/gist/gistscan.c
@@ -89,11 +89,9 @@ gistbeginscan(PG_FUNCTION_ARGS)
 	scan->opaque = so;
 
 	/*
-	 * All fields required for index-only scans are null until gistrescan.
-	 * However, we set up scan->xs_itupdesc whether we'll need it or not,
-	 * since that's cheap.
+	 * All fields required for index-only scans are initialized in gistrescan,
+	 * as we don't know yet if we're doing an index-only scan or not.
 	 */
-	scan->xs_itupdesc = RelationGetDescr(r);
 
 	MemoryContextSwitchTo(oldCxt);
 
@@ -149,15 +147,37 @@ gistrescan(PG_FUNCTION_ARGS)
 	}
 
 	/*
-	 * If we're doing an index-only scan, also create a memory context to hold
-	 * the returned tuples.
+	 * If we're doing an index-only scan, on the first call, also initialize
+	 * a tuple descriptor to represent the returned index tuples and create a
+	 * memory context to hold them during the scan.
 	 */
-	if (scan->xs_want_itup && so->pageDataCxt == NULL)
+	if (scan->xs_want_itup && !scan->xs_itupdesc)
+	{
+		int			natts;
+		int			attno;
+
+		/*
+		 * The storage type of the index can be different from the original
+		 * datatype being indexed, so we cannot just grab the index's tuple
+		 * descriptor. Instead, construct a descriptor with the original data
+		 * types.
+		 */
+		natts =  RelationGetNumberOfAttributes(scan->indexRelation);
+		so->giststate->fetchTupdesc = CreateTemplateTupleDesc(natts, false);
+		for (attno = 1; attno <= natts; attno++)
+		{
+			TupleDescInitEntry(so->giststate->fetchTupdesc, attno, NULL,
+							   scan->indexRelation->rd_opcintype[attno - 1],
+							   -1, 0);
+		}
+		scan->xs_itupdesc = so->giststate->fetchTupdesc;
+
 		so->pageDataCxt = AllocSetContextCreate(so->giststate->scanCxt,
 												"GiST page data context",
 												ALLOCSET_DEFAULT_MINSIZE,
 												ALLOCSET_DEFAULT_INITSIZE,
 												ALLOCSET_DEFAULT_MAXSIZE);
+	}
 
 	/* create new, empty RBTree for search queue */
 	oldCxt = MemoryContextSwitchTo(so->queueCxt);
diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c
@@ -657,7 +657,7 @@ gistFetchTuple(GISTSTATE *giststate, Relation r, IndexTuple tuple)
 	}
 	MemoryContextSwitchTo(oldcxt);
 
-	return index_form_tuple(giststate->tupdesc, fetchatt, isnull);
+	return index_form_tuple(giststate->fetchTupdesc, fetchatt, isnull);
 }
 
 float
diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h
@@ -78,6 +78,8 @@ typedef struct GISTSTATE
 	MemoryContext tempCxt;		/* short-term context for calling functions */
 
 	TupleDesc	tupdesc;		/* index's tuple descriptor */
+	TupleDesc	fetchTupdesc;	/* tuple descriptor for tuples returned in an
+								 * index-only scan */
 
 	FmgrInfo	consistentFn[INDEX_MAX_KEYS];
 	FmgrInfo	unionFn[INDEX_MAX_KEYS];

Original file line number	Diff line number	Diff line change
`@@ -657,7 +657,7 @@ gistFetchTuple(GISTSTATE *giststate, Relation r, IndexTuple tuple)`
`657`	`657`	`}`
`658`	`658`	`MemoryContextSwitchTo(oldcxt);`
`659`	`659`
`660`		`- return index_form_tuple(giststate->tupdesc, fetchatt, isnull);`
	`660`	`+ return index_form_tuple(giststate->fetchTupdesc, fetchatt, isnull);`
`661`	`661`	`}`
`662`	`662`
`663`	`663`	`float`