From 5fc7b472ca6e335a83fab81ca767d074279ff969 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Fri, 27 Sep 2019 00:17:38 -0700
Subject: [PATCH v1 10/12] jit: Fix pessimization of execGrouping.c
 comparisons.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/executor/execGrouping.c | 13 ++++++++++++-
 src/test/regress/expected/jit.out   |  8 +++-----
 src/test/regress/sql/jit.sql        |  2 --
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/backend/executor/execGrouping.c b/src/backend/executor/execGrouping.c
index 14ee8db3f98..6349c11e1d5 100644
--- a/src/backend/executor/execGrouping.c
+++ b/src/backend/executor/execGrouping.c
@@ -166,6 +166,7 @@ BuildTupleHashTableExt(PlanState *parent,
 	TupleHashTable hashtable;
 	Size		entrysize = sizeof(TupleHashEntryData) + additionalsize;
 	MemoryContext oldcontext;
+	bool		allow_jit;
 
 	Assert(nbuckets > 0);
 
@@ -210,13 +211,23 @@ BuildTupleHashTableExt(PlanState *parent,
 	hashtable->tableslot = MakeSingleTupleTableSlot(CreateTupleDescCopy(inputDesc),
 													&TTSOpsMinimalTuple);
 
+	/*
+	 * If the old reset interface is used (i.e. BuildTupleHashTable, rather
+	 * than BuildTupleHashTableExt), allowing JIT would lead to the generated
+	 * functions to a) live longer than the query b) be re-generated each time
+	 * the table is being reset. Therefore prevent JIT from being used in that
+	 * case, by not providing a parent node (which prevents accessing the
+	 * JitContext in the EState).
+	 */
+	allow_jit = metacxt != tablecxt;
+
 	/* build comparator for all columns */
 	/* XXX: should we support non-minimal tuples for the inputslot? */
 	hashtable->tab_eq_func = ExecBuildGroupingEqual(inputDesc, inputDesc,
 													&TTSOpsMinimalTuple, &TTSOpsMinimalTuple,
 													numCols,
 													keyColIdx, eqfuncoids, collations,
-													NULL);
+													allow_jit ? parent : NULL);
 
 	/*
 	 * While not pretty, it's ok to not shut down this context, but instead
diff --git a/src/test/regress/expected/jit.out b/src/test/regress/expected/jit.out
index 4db4ae6d352..151faaa2fde 100644
--- a/src/test/regress/expected/jit.out
+++ b/src/test/regress/expected/jit.out
@@ -418,8 +418,6 @@ SELECT count(*), count(data), string_agg(data, ':') FROM jittest_simple;
 
 -- Check that the equality hash-table function in a hash-aggregate can
 -- be accelerated.
---
--- XXX: Unfortunately this is currently broken
 BEGIN;
 SET LOCAL enable_hashagg = true;
 SET LOCAL enable_sort = false;
@@ -429,12 +427,12 @@ EXPLAIN (VERBOSE, COSTS OFF, JIT_DETAILS) SELECT data, string_agg(id::text, ', '
  HashAggregate
    Project: data, string_agg((id)::text, ', '::text); JIT-Expr: evalexpr_0_0, JIT-Deform-Outer: deform_0_1
    Phase 0 using strategy "Hash":
-     Transition Function: string_agg_transfn(TRANS, (id)::text, ', '::text); JIT-Expr: evalexpr_0_2, JIT-Deform-Outer: deform_0_3
-     Hash Group: jittest_simple.data; JIT-Expr: false
+     Transition Function: string_agg_transfn(TRANS, (id)::text, ', '::text); JIT-Expr: evalexpr_0_5, JIT-Deform-Outer: deform_0_6
+     Hash Group: jittest_simple.data; JIT-Expr: evalexpr_0_2, JIT-Deform-Outer: deform_0_4, JIT-Deform-Inner: deform_0_3
    ->  Seq Scan on public.jittest_simple
          Output: id, data
  JIT:
-   Functions: 4 (2 for expression evaluation, 2 for tuple deforming)
+   Functions: 7 (3 for expression evaluation, 4 for tuple deforming)
    Options: Inlining false, Optimization false, Expressions true, Deforming true
 (10 rows)
 
diff --git a/src/test/regress/sql/jit.sql b/src/test/regress/sql/jit.sql
index f3b9a352cf1..eb617c0ca58 100644
--- a/src/test/regress/sql/jit.sql
+++ b/src/test/regress/sql/jit.sql
@@ -144,8 +144,6 @@ SELECT count(*), count(data), string_agg(data, ':') FROM jittest_simple;
 
 -- Check that the equality hash-table function in a hash-aggregate can
 -- be accelerated.
---
--- XXX: Unfortunately this is currently broken
 BEGIN;
 SET LOCAL enable_hashagg = true;
 SET LOCAL enable_sort = false;
-- 
2.23.0.162.gf1d4a28250

