bugfix: RIGHT OUTER tuple didn't check 'other_quals' in 'join_quals'

it was reported at #827
heterodb · Oct 15, 2024 · cb2c4e7 · cb2c4e7
1 parent 9389fb7
commit cb2c4e7
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 19 deletions.
diff --git a/src/cuda_gpujoin.cu b/src/cuda_gpujoin.cu
@@ -245,20 +245,19 @@ execGpuJoinNestLoop(kern_context *kcxt,
 		{
 			kern_tupitem *tupitem;
 			uint64_t	offset = KDS_GET_ROWINDEX(kds_heap)[index];
-			xpu_int4_t	status;
+			int			status;
 
 			tupitem = (kern_tupitem *)((char *)kds_heap
 									   + kds_heap->length
 									   - offset);
 			kexp = SESSION_KEXP_LOAD_VARS(kcxt->session, depth);
 			ExecLoadVarsHeapTuple(kcxt, kexp, depth, kds_heap, &tupitem->htup);
 			kexp = SESSION_KEXP_JOIN_QUALS(kcxt->session, depth);
-			if (EXEC_KERN_EXPRESSION(kcxt, kexp, &status))
+			if (ExecGpuJoinQuals(kcxt, kexp, &status))
 			{
-				assert(!XPU_DATUM_ISNULL(&status));
-				if (status.value > 0)
+				if (status > 0)
 					tuple_is_valid = true;
-				if (status.value != 0)
+				if (status != 0)
 				{
 					assert(tupitem->rowid < kds_heap->nitems);
 					if (oj_map)
@@ -273,10 +272,19 @@ execGpuJoinNestLoop(kern_context *kcxt,
 		}
 		else if (left_outer && index >= kds_heap->nitems && !matched)
 		{
+			bool		status;
 			/* fill up NULL fields, if FULL/LEFT OUTER JOIN */
 			kexp = SESSION_KEXP_LOAD_VARS(kcxt->session, depth);
 			ExecLoadVarsHeapTuple(kcxt, kexp, depth, kds_heap, NULL);
-			tuple_is_valid = true;
+			kexp = SESSION_KEXP_JOIN_QUALS(kcxt->session, depth);
+			if (ExecGpuJoinOtherQuals(kcxt, kexp, &status))
+			{
+				tuple_is_valid = status;
+			}
+			else
+			{
+				HandleErrorIfCpuFallback(kcxt, depth, index, matched);
+			}
 			l_state = ULONG_MAX;
 		}
 		else
@@ -438,18 +446,17 @@ execGpuJoinHashJoin(kern_context *kcxt,
 
 	if (khitem)
 	{
-		xpu_int4_t	status;
+		int		status;
 
 		l_state = ((char *)khitem - (char *)kds_hash);
 		kexp = SESSION_KEXP_LOAD_VARS(kcxt->session, depth);
 		ExecLoadVarsHeapTuple(kcxt, kexp, depth, kds_hash, &khitem->t.htup);
 		kexp = SESSION_KEXP_JOIN_QUALS(kcxt->session, depth);
-		if (EXEC_KERN_EXPRESSION(kcxt, kexp, &status))
+		if (ExecGpuJoinQuals(kcxt, kexp, &status))
 		{
-			assert(!XPU_DATUM_ISNULL(&status));
-			if (status.value > 0)
+			if (status > 0)
 				tuple_is_valid = true;
-			if (status.value != 0)
+			if (status != 0)
 			{
 				assert(khitem->t.rowid < kds_hash->nitems);
 				if (oj_map)
@@ -467,10 +474,19 @@ execGpuJoinHashJoin(kern_context *kcxt,
 		if (kmrels->chunks[depth-1].left_outer &&
 			l_state != ULONG_MAX && !matched)
 		{
+			bool	status;
 			/* load NULL values on the inner portion */
-			 kexp = SESSION_KEXP_LOAD_VARS(kcxt->session, depth);
-			 ExecLoadVarsHeapTuple(kcxt, kexp, depth, kds_hash, NULL);
-			 tuple_is_valid = true;
+			kexp = SESSION_KEXP_LOAD_VARS(kcxt->session, depth);
+			ExecLoadVarsHeapTuple(kcxt, kexp, depth, kds_hash, NULL);
+			kexp = SESSION_KEXP_JOIN_QUALS(kcxt->session, depth);
+			if (ExecGpuJoinOtherQuals(kcxt, kexp, &status))
+			{
+				tuple_is_valid = status;
+			}
+			else
+			{
+				HandleErrorIfCpuFallback(kcxt, depth, l_state, matched);
+			}
 		}
 		l_state = ULONG_MAX;
 	}

diff --git a/src/xpu_common.cu b/src/xpu_common.cu
@@ -1165,12 +1165,28 @@ pgfn_SaveExpr(XPU_PGFUNCTION_ARGS)
 
 STATIC_FUNCTION(bool)
 pgfn_JoinQuals(XPU_PGFUNCTION_ARGS)
+{
+	STROM_ELOG(kcxt, "pgfn_JoinQuals should not be called as a normal kernel expression");
+	return false;
+}
+
+/*
+ * ExecGpuJoinQuals - runs JoinQuals operator.
+ *   result =  1 : matched to JoinQuals
+ *          =  0 : unmatched to JoinQuals
+ *          = -1 : matched to JoinQuals, but unmatched to any of other quals
+ *                 --> don't generate inner join row, but set outer-join-map.
+ */
+PUBLIC_FUNCTION(bool)
+ExecGpuJoinQuals(kern_context *kcxt,
+				 const kern_expression *kexp,
+				 int *p_status)
 {
 	const kern_expression *karg;
-	xpu_int4_t *result = (xpu_int4_t *)__result;
-	int			i, status = 1;
+	int		i, status = 1;
 
-	assert(kexp->exptype == TypeOpCode__bool);
+	assert(kexp->opcode  == FuncOpCode__JoinQuals &&
+		   kexp->exptype == TypeOpCode__bool);
 	for (i=0, karg = KEXP_FIRST_ARG(kexp);
 		 i < kexp->nr_args;
 		 i++, karg = KEXP_NEXT_ARG(karg))
@@ -1198,8 +1214,42 @@ pgfn_JoinQuals(XPU_PGFUNCTION_ARGS)
 			status = -1;
 		}
 	}
-	result->expr_ops = kexp->expr_ops;
-	result->value = status;
+	*p_status = status;
+	return true;
+}
+
+/*
+ * ExecGpuJoinOtherQuals - runs OtherQuals in the JoinQuals if any.
+ *   it is usually used in validation of RIGHT OUTER JOIN row.
+ */
+PUBLIC_FUNCTION(bool)
+ExecGpuJoinOtherQuals(kern_context *kcxt,
+					  const kern_expression *kexp,
+					  bool *p_status)
+{
+	const kern_expression *karg;
+	bool	status = true;
+	int		i;
+
+	assert(kexp->opcode  == FuncOpCode__JoinQuals &&
+		   kexp->exptype == TypeOpCode__bool);
+	for (i=0, karg = KEXP_FIRST_ARG(kexp);
+		 i < kexp->nr_args;
+		 i++, karg = KEXP_NEXT_ARG(karg))
+	{
+		xpu_bool_t	datum;
+
+		if ((karg->expflags & KEXP_FLAG__IS_PUSHED_DOWN) == 0)
+			continue;
+		if (!EXEC_KERN_EXPRESSION(kcxt, karg, &datum))
+			return false;
+		if (XPU_DATUM_ISNULL(&datum) || !datum.value)
+		{
+			status = false;
+			break;
+		}
+	}
+	*p_status = status;
 	return true;
 }
 

diff --git a/src/xpu_common.h b/src/xpu_common.h
@@ -2985,6 +2985,14 @@ ExecMoveKernelVariables(kern_context *kcxt,
 						const kern_expression *kexp_move_vars,
                         char *dst_kvec_buffer,
                         int dst_kvec_id);
+EXTERN_FUNCTION(bool)
+ExecGpuJoinQuals(kern_context *kcxt,
+				 const kern_expression *kexp_join_quals,
+				 int *p_status);
+EXTERN_FUNCTION(bool)
+ExecGpuJoinOtherQuals(kern_context *kcxt,
+					  const kern_expression *kexp_join_quals,
+					  bool *p_status);
 EXTERN_FUNCTION(uint64_t)
 ExecGiSTIndexGetNext(kern_context *kcxt,
 					 const kern_data_store *kds_hash,