bugfix: incorrect inner hash-join key reference

reported at heterodb#662
charliettxx · Nov 18, 2023 · 854189c · 854189c
1 parent 268cfd3
commit 854189c
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 61 deletions.
diff --git a/src/executor.c b/src/executor.c
@@ -1284,7 +1284,7 @@ __fixup_fallback_projection(Node *node, void *__data)
 		if (equal(kvdef->kv_expr, node))
 		{
 			return (Node *)makeVar(INDEX_VAR,
-								   kvdef->kv_slot_id,
+								   kvdef->kv_slot_id + 1,
 								   exprType(node),
 								   exprTypmod(node),
 								   exprCollation(node),
@@ -1306,50 +1306,6 @@ __fixup_fallback_projection(Node *node, void *__data)
 /*
  * fixup_fallback_join_inner_keys
  */
-typedef struct
-{
-	List   *kvars_deflist;
-	int		inner_depth;
-} fixup_fallback_join_inner_keys_context;
-
-static Node *
-__fixup_fallback_join_inner_keys_walker(Node *node, void *__data)
-{
-	fixup_fallback_join_inner_keys_context *con = __data;
-
-	if (!node)
-		return NULL;
-	if (IsA(node, Var))
-	{
-		codegen_kvar_defitem *kvdef;
-		Var	   *var = (Var *)node;
-
-		Assert(var->varno == INDEX_VAR &&
-			   var->varattno >= 0 &&
-			   var->varattno < list_length(con->kvars_deflist));
-		kvdef = list_nth(con->kvars_deflist, var->varattno);
-		Assert(kvdef->kv_depth == con->inner_depth);
-		return (Node *)makeVar(INNER_VAR,
-							   kvdef->kv_resno,
-							   exprType(node),
-							   exprTypmod(node),
-							   exprCollation(node),
-							   0);
-	}
-	return expression_tree_mutator(node, __fixup_fallback_join_inner_keys_walker, __data);
-}
-
-static Node *
-fixup_fallback_join_inner_keys(Node *inner_key, pgstromPlanInfo *pp_info, int inner_depth)
-{
-	fixup_fallback_join_inner_keys_context con;
-
-	memset(&con, 0, sizeof(con));
-	con.kvars_deflist = pp_info->kvars_deflist;
-	con.inner_depth   = inner_depth;
-	return __fixup_fallback_join_inner_keys_walker(inner_key, &con);
-}
-
 static void
 __execInitTaskStateCpuFallback(pgstromTaskState *pts)
 {
@@ -1555,7 +1511,8 @@ pgstromExecInitTaskState(CustomScanState *node, EState *estate, int eflags)
 					 nodeToString(outer_key));
 			es = ExecInitExpr((Expr *)outer_key, &pts->css.ss.ps);
 			istate->hash_outer_keys = lappend(istate->hash_outer_keys, es);
-			istate->hash_outer_dtypes = lappend(istate->hash_outer_dtypes, dtype);
+			istate->hash_outer_funcs = lappend(istate->hash_outer_funcs,
+											   dtype->type_hashfunc);
 		}
 		/* inner hash-keys references the result of inner-slot */
 		foreach (cell, pp_inner->hash_inner_keys_fallback)
@@ -1568,12 +1525,10 @@ pgstromExecInitTaskState(CustomScanState *node, EState *estate, int eflags)
 			if (!dtype)
 				elog(ERROR, "failed on lookup device type of %s",
 					 nodeToString(inner_key));
-			inner_key = fixup_fallback_join_inner_keys(inner_key,
-													   pp_info,
-													   istate->depth);
 			es = ExecInitExpr((Expr *)inner_key, &pts->css.ss.ps);
 			istate->hash_inner_keys = lappend(istate->hash_inner_keys, es);
-			istate->hash_inner_dtypes = lappend(istate->hash_inner_dtypes, dtype);
+			istate->hash_inner_funcs = lappend(istate->hash_inner_funcs,
+											   dtype->type_hashfunc);
 		}
 
 		if (OidIsValid(pp_inner->gist_index_oid))

diff --git a/src/gpu_join.c b/src/gpu_join.c
@@ -732,7 +732,7 @@ __build_fallback_exprs_join_walker(Node *node, void *data)
 		if (equal(node, kvar->kv_expr))
 		{
 			return (Node *)makeVar(INDEX_VAR,
-								   kvar->kv_slot_id,
+								   kvar->kv_slot_id + 1,
 								   exprType(node),
 								   exprTypmod(node),
 								   exprCollation(node),
@@ -743,7 +743,7 @@ __build_fallback_exprs_join_walker(Node *node, void *data)
 		elog(ERROR, "Bug? Var-node (%s) is missing at the kvars_exprs list",
 			 nodeToString(node));
 
-	return expression_tree_mutator(node, __build_fallback_exprs_join_walker, context);
+	return expression_tree_mutator(node, __build_fallback_exprs_join_walker, data);
 }
 
 static List *
@@ -752,6 +752,41 @@ build_fallback_exprs_join(codegen_context *context, List *join_exprs)
 	return (List *)__build_fallback_exprs_join_walker((Node *)join_exprs, context);
 }
 
+static Node *
+__build_fallback_exprs_inner_walker(Node *node, void *data)
+{
+	codegen_context *context = (codegen_context *)data;
+	ListCell   *lc;
+
+	if (!node)
+		return NULL;
+	foreach (lc, context->kvars_deflist)
+	{
+		codegen_kvar_defitem *kvar = lfirst(lc);
+
+		if (equal(node, kvar->kv_expr))
+		{
+			return (Node *)makeVar(INNER_VAR,
+								   kvar->kv_resno,
+								   exprType(node),
+								   exprTypmod(node),
+								   exprCollation(node),
+								   0);
+		}
+	}
+	if (IsA(node, Var))
+		elog(ERROR, "Bug? Var-node (%s) is missing at the kvars_exprs list",
+			 nodeToString(node));
+
+	return expression_tree_mutator(node, __build_fallback_exprs_inner_walker, data);
+}
+
+static List *
+build_fallback_exprs_inner(codegen_context *context, List *inner_keys)
+{
+	return (List *)__build_fallback_exprs_inner_walker((Node *)inner_keys, context);
+}
+
 /*
  * pgstrom_build_tlist_dev
  */
@@ -1142,7 +1177,7 @@ PlanXpuJoinPathCommon(PlannerInfo *root,
 		pp_inner->hash_outer_keys_fallback
 			= build_fallback_exprs_join(context, pp_inner->hash_outer_keys);
 		pp_inner->hash_inner_keys_fallback
-			= build_fallback_exprs_join(context, pp_inner->hash_inner_keys);
+			= build_fallback_exprs_inner(context, pp_inner->hash_inner_keys);
 		pp_inner->join_quals_fallback
 			= build_fallback_exprs_join(context, pp_inner->join_quals);
 		pp_inner->other_quals_fallback
@@ -1298,15 +1333,15 @@ get_tuple_hashvalue(pgstromTaskInnerState *istate,
 	/* calculation of a hash value of this entry */
 	econtext->ecxt_innertuple = slot;
 	forboth (lc1, istate->hash_inner_keys,
-			 lc2, istate->hash_inner_dtypes)
+			 lc2, istate->hash_inner_funcs)
 	{
 		ExprState	   *es = lfirst(lc1);
-		devtype_info   *dtype = lfirst(lc2);
+		devtype_hashfunc_f h_func = lfirst(lc2);
 		Datum			datum;
 		bool			isnull;
 
 		datum = ExecEvalExpr(es, econtext, &isnull);
-		hash ^= dtype->type_hashfunc(isnull, datum);
+		hash ^= h_func(isnull, datum);
 	}
 	hash ^= 0xffffffffU;
 
@@ -2090,15 +2125,15 @@ __execFallbackCpuHashJoin(pgstromTaskState *pts,
 	 */
 	hash = 0xffffffffU;
 	forboth (lc1, istate->hash_outer_keys,
-			 lc2, istate->hash_outer_dtypes)
+			 lc2, istate->hash_outer_funcs)
 	{
 		ExprState	   *h_key = lfirst(lc1);
-		devtype_info   *dtype = lfirst(lc2);
+		devtype_hashfunc_f h_func = lfirst(lc2);
 		Datum			datum;
 		bool			isnull;
 
 		datum = ExecEvalExprSwitchContext(h_key, econtext, &isnull);
-		hash ^= dtype->type_hashfunc(isnull, datum);
+		hash ^= h_func(isnull, datum);
 	}
 	hash ^= 0xffffffffU;
 

diff --git a/src/pg_strom.h b/src/pg_strom.h
@@ -406,8 +406,8 @@ typedef struct
 	 */
 	List		   *hash_outer_keys;    /* list of ExprState */
 	List		   *hash_inner_keys;    /* list of ExprState */
-	List		   *hash_outer_dtypes;  /* list of devtype_info */
-	List		   *hash_inner_dtypes;  /* list of devtype_info */
+	List		   *hash_outer_funcs;	/* list of devtype_hashfunc_f */
+	List		   *hash_inner_funcs;	/* list of devtype_hashfunc_f */
 	/*
 	 * join properties (gist-join)
 	 */