From 39e2a404909e34de82a17be9110dc9f42a38823c Mon Sep 17 00:00:00 2001
From: Colm McHugh <cmchugh@salesforce.com>
Date: Fri, 17 May 2019 15:32:52 -0700
Subject: [PATCH] Fix write after end of array in hashed Agg initialization.

The array for the column indices in the per-hashtable data structure
may be under-allocated in some corner cases, resulting in a write
after the end of this array. Using the number of grouping columns and
number of other columns in the qualifier and targetlist (if any) to
size the array avoids the corner cases where this can happen.
---
 src/backend/executor/nodeAgg.c           | 28 +++++++++++++++++-----------
 src/test/regress/expected/aggregates.out | 27 +++++++++++++++++++++++++++
 src/test/regress/sql/aggregates.sql      |  4 ++++
 3 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index d01fc4f..079bffd 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -1331,14 +1331,23 @@ find_hash_columns(AggState *aggstate)
 	for (j = 0; j < numHashes; ++j)
 	{
 		AggStatePerHash perhash = &aggstate->perhash[j];
-		Bitmapset  *colnos = bms_copy(base_colnos);
+		Bitmapset  *grouping_colnos = NULL;
+		Bitmapset  *non_grouping_colnos = NULL;
 		AttrNumber *grpColIdx = perhash->aggnode->grpColIdx;
 		List	   *hashTlist = NIL;
 		TupleDesc	hashDesc;
 		int			i;
+		int			hashGrpColIdxSize = 0;
 
 		perhash->largestGrpColIdx = 0;
 
+		/* Populate grouping_colnos - this is the set of all the grouping columns */
+		for (i = 0; i < perhash->numCols; i++)
+			grouping_colnos = bms_add_member(grouping_colnos, grpColIdx[i]);
+
+		/* Determine remaining columns (if any) */
+		non_grouping_colnos = bms_difference(base_colnos, grouping_colnos);
+
 		/*
 		 * If we're doing grouping sets, then some Vars might be referenced in
 		 * tlist/qual for the benefit of other grouping sets, but not needed
@@ -1356,15 +1365,13 @@ find_hash_columns(AggState *aggstate)
 				int			attnum = lfirst_int(lc);
 
 				if (!bms_is_member(attnum, grouped_cols))
-					colnos = bms_del_member(colnos, attnum);
+					non_grouping_colnos = bms_del_member(non_grouping_colnos, attnum);
 			}
 		}
-		/* Add in all the grouping columns */
-		for (i = 0; i < perhash->numCols; i++)
-			colnos = bms_add_member(colnos, grpColIdx[i]);
-
+		/* allocate a slot for each grouping column and remaining column */
+		hashGrpColIdxSize = perhash->numCols + bms_num_members(non_grouping_colnos);
 		perhash->hashGrpColIdxInput =
-			palloc(bms_num_members(colnos) * sizeof(AttrNumber));
+			palloc(hashGrpColIdxSize * sizeof(AttrNumber));
 		perhash->hashGrpColIdxHash =
 			palloc(perhash->numCols * sizeof(AttrNumber));
 
@@ -1380,12 +1387,10 @@ find_hash_columns(AggState *aggstate)
 			perhash->hashGrpColIdxInput[i] = grpColIdx[i];
 			perhash->hashGrpColIdxHash[i] = i + 1;
 			perhash->numhashGrpCols++;
-			/* delete already mapped columns */
-			bms_del_member(colnos, grpColIdx[i]);
 		}
 
 		/* and add the remaining columns */
-		while ((i = bms_first_member(colnos)) >= 0)
+		while ((i = bms_first_member(non_grouping_colnos)) >= 0)
 		{
 			perhash->hashGrpColIdxInput[perhash->numhashGrpCols] = i;
 			perhash->numhashGrpCols++;
@@ -1412,7 +1417,8 @@ find_hash_columns(AggState *aggstate)
 							   &TTSOpsMinimalTuple);
 
 		list_free(hashTlist);
-		bms_free(colnos);
+		bms_free(grouping_colnos);
+		bms_free(non_grouping_colnos);
 	}
 
 	bms_free(base_colnos);
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 129c1e5..3ef7ac9 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -2265,3 +2265,30 @@ select v||'a', case when v||'a' = 'aa' then 1 else 0 end, count(*)
  ba       |    0 |     1
 (2 rows)
 
+explain (costs off) delete from tenk1 where (hundred, thousand) in (select twothousand, twothousand from onek);
+                            QUERY PLAN                             
+-------------------------------------------------------------------
+ Delete on tenk1
+   ->  Hash Join
+         Hash Cond: (tenk1.hundred = onek.twothousand)
+         ->  Seq Scan on tenk1
+               Filter: (hundred = thousand)
+         ->  Hash
+               ->  HashAggregate
+                     Group Key: onek.twothousand, onek.twothousand
+                     ->  Seq Scan on onek
+(9 rows)
+
+explain (costs off) select 1 from tenk1 where (hundred, thousand) in (select twothousand, twothousand from onek); 
+                         QUERY PLAN                          
+-------------------------------------------------------------
+ Hash Join
+   Hash Cond: (tenk1.hundred = onek.twothousand)
+   ->  Seq Scan on tenk1
+         Filter: (hundred = thousand)
+   ->  Hash
+         ->  HashAggregate
+               Group Key: onek.twothousand, onek.twothousand
+               ->  Seq Scan on onek
+(8 rows)
+
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index d4fd657..5bd77d4 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -987,3 +987,7 @@ select v||'a', case v||'a' when 'aa' then 1 else 0 end, count(*)
 select v||'a', case when v||'a' = 'aa' then 1 else 0 end, count(*)
   from unnest(array['a','b']) u(v)
  group by v||'a' order by 1;
+
+explain (costs off) delete from tenk1 where (hundred, thousand) in (select twothousand, twothousand from onek);
+
+explain (costs off) select 1 from tenk1 where (hundred, thousand) in (select twothousand, twothousand from onek);
-- 
2.7.4

