From cb2c4e7a1a6a39e6d81e0bc8dceef9c8e54f0a35 Mon Sep 17 00:00:00 2001 From: KaiGai Kohei Date: Tue, 15 Oct 2024 23:12:35 +0900 Subject: [PATCH] bugfix: RIGHT OUTER tuple didn't check 'other_quals' in 'join_quals' it was reported at #827 --- src/cuda_gpujoin.cu | 44 ++++++++++++++++++++++----------- src/xpu_common.cu | 60 +++++++++++++++++++++++++++++++++++++++++---- src/xpu_common.h | 8 ++++++ 3 files changed, 93 insertions(+), 19 deletions(-) diff --git a/src/cuda_gpujoin.cu b/src/cuda_gpujoin.cu index 8c747f3e..2824b5e5 100644 --- a/src/cuda_gpujoin.cu +++ b/src/cuda_gpujoin.cu @@ -245,7 +245,7 @@ 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 @@ -253,12 +253,11 @@ execGpuJoinNestLoop(kern_context *kcxt, 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 index 5b8cf17c..ef884200 100644 --- 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 index dbd19c94..662e0628 100644 --- 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,