Have crosstab variants treat NULL rowid as a category in its own right,

per suggestion from Tom Lane. This fixes crash-bug reported by Stefan
Schwarzer.

1 files changed, 57 insertions, 35 deletions

diff --git a/contrib/tablefunc/tablefunc.c b/contrib/tablefunc/tablefunc.c
index f3b1eb2a8b4..22dc2f2e0eb 100644
--- a/contrib/tablefunc/tablefunc.c
+++ b/contrib/tablefunc/tablefunc.c
@@ -106,6 +106,18 @@ typedef struct
 		} \
 	} while (0)
 
+#define xpstrdup(tgtvar_, srcvar_) \
+	do { \
+		if (srcvar_) \
+			tgtvar_ = pstrdup(srcvar_); \
+		else \
+			tgtvar_ = NULL; \
+	} while (0)
+
+#define xstreq(tgtvar_, srcvar_) \
+	(((tgtvar_ == NULL) && (srcvar_ == NULL)) || \
+	 ((tgtvar_ != NULL) && (srcvar_ != NULL) && (strcmp(tgtvar_, srcvar_) == 0)))
+
 /* sign, 10 digits, '\0' */
 #define INT32_STRLEN	12
 
@@ -355,6 +367,7 @@ crosstab(PG_FUNCTION_ARGS)
 	crosstab_fctx *fctx;
 	int			i;
 	int			num_categories;
+	bool		firstpass = false;
 	MemoryContext oldcontext;
 
 	/* stuff done only on the first call of the function */
@@ -469,6 +482,7 @@ crosstab(PG_FUNCTION_ARGS)
 		funcctx->max_calls = proc;
 
 		MemoryContextSwitchTo(oldcontext);
+		firstpass = true;
 	}
 
 	/* stuff done on every call of the function */
@@ -500,7 +514,7 @@ crosstab(PG_FUNCTION_ARGS)
 		HeapTuple	tuple;
 		Datum		result;
 		char	  **values;
-		bool		allnulls = true;
+		bool		skip_tuple = false;
 
 		while (true)
 		{
@@ -530,26 +544,35 @@ crosstab(PG_FUNCTION_ARGS)
 				rowid = SPI_getvalue(spi_tuple, spi_tupdesc, 1);
 
 				/*
-				 * If this is the first pass through the values for this rowid
-				 * set it, otherwise make sure it hasn't changed on us. Also
-				 * check to see if the rowid is the same as that of the last
-				 * tuple sent -- if so, skip this tuple entirely
+				 * If this is the first pass through the values for this
+				 * rowid, set the first column to rowid
 				 */
 				if (i == 0)
-					values[0] = pstrdup(rowid);
-
-				if ((rowid != NULL) && (strcmp(rowid, values[0]) == 0))
 				{
-					if ((lastrowid != NULL) && (strcmp(rowid, lastrowid) == 0))
+					xpstrdup(values[0], rowid);
+
+					/*
+					 * Check to see if the rowid is the same as that of the last
+					 * tuple sent -- if so, skip this tuple entirely
+					 */
+					if (!firstpass && xstreq(lastrowid, rowid))
+					{
+						skip_tuple = true;
 						break;
-					else if (allnulls == true)
-						allnulls = false;
+					}
+				}
 
+				/*
+				 * If rowid hasn't changed on us, continue building the
+				 * ouput tuple.
+				 */
+				if (xstreq(rowid, values[0]))
+				{
 					/*
-					 * Get the next category item value, which is alway
+					 * Get the next category item value, which is always
 					 * attribute number three.
 					 *
-					 * Be careful to sssign the value to the array index based
+					 * Be careful to assign the value to the array index based
 					 * on which category we are presently processing.
 					 */
 					values[1 + i] = SPI_getvalue(spi_tuple, spi_tupdesc, 3);
@@ -572,26 +595,22 @@ crosstab(PG_FUNCTION_ARGS)
 					call_cntr = --funcctx->call_cntr;
 					break;
 				}
-
-				if (rowid != NULL)
-					xpfree(rowid);
+				xpfree(rowid);
 			}
 
-			xpfree(fctx->lastrowid);
+			/*
+			 * switch to memory context appropriate for multiple function
+			 * calls
+			 */
+			oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
 
-			if (values[0] != NULL)
-			{
-				/*
-				 * switch to memory context appropriate for multiple function
-				 * calls
-				 */
-				oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+			xpfree(fctx->lastrowid);
+			xpstrdup(fctx->lastrowid, values[0]);
+			lastrowid = fctx->lastrowid;
 
-				lastrowid = fctx->lastrowid = pstrdup(values[0]);
-				MemoryContextSwitchTo(oldcontext);
-			}
+			MemoryContextSwitchTo(oldcontext);
 
-			if (!allnulls)
+			if (!skip_tuple)
 			{
 				/* build the tuple */
 				tuple = BuildTupleFromCStrings(attinmeta, values);
@@ -625,6 +644,9 @@ crosstab(PG_FUNCTION_ARGS)
 					SPI_finish();
 					SRF_RETURN_DONE(funcctx);
 				}
+
+				/* need to reset this before the next tuple is started */
+				skip_tuple = false;
 			}
 		}
 	}
@@ -856,6 +878,7 @@ get_crosstab_tuplestore(char *sql,
 		int			ncols = spi_tupdesc->natts;
 		char	   *rowid;
 		char	   *lastrowid = NULL;
+		bool		firstpass = true;
 		int			i,
 					j;
 		int			result_ncols;
@@ -918,21 +941,17 @@ get_crosstab_tuplestore(char *sql,
 			/* get the rowid from the current sql result tuple */
 			rowid = SPI_getvalue(spi_tuple, spi_tupdesc, 1);
 
-			/* if rowid is null, skip this tuple entirely */
-			if (rowid == NULL)
-				continue;
-
 			/*
 			 * if we're on a new output row, grab the column values up to
 			 * column N-2 now
 			 */
-			if ((lastrowid == NULL) || (strcmp(rowid, lastrowid) != 0))
+			if (firstpass || !xstreq(lastrowid, rowid))
 			{
 				/*
 				 * a new row means we need to flush the old one first, unless
 				 * we're on the very first row
 				 */
-				if (lastrowid != NULL)
+				if (!firstpass)
 				{
 					/* rowid changed, flush the previous output row */
 					tuple = BuildTupleFromCStrings(attinmeta, values);
@@ -949,6 +968,9 @@ get_crosstab_tuplestore(char *sql,
 				values[0] = rowid;
 				for (j = 1; j < ncols - 2; j++)
 					values[j] = SPI_getvalue(spi_tuple, spi_tupdesc, j + 1);
+
+				/* we're no longer on the first pass */
+				firstpass = false;
 			}
 
 			/* look up the category and fill in the appropriate column */
@@ -964,7 +986,7 @@ get_crosstab_tuplestore(char *sql,
 			}
 
 			xpfree(lastrowid);
-			lastrowid = pstrdup(rowid);
+			xpstrdup(lastrowid, rowid);
 		}
 
 		/* flush the last output row */