From 92ef364a8f650022a139bc32a2e518804a41767a Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Wed, 8 Oct 2025 08:06:59 -0400 Subject: [PATCH v3 9/9] Blind guess at fixing segfault on running tpch q22 --- src/backend/executor/execExprInterp.c | 225 ++++++++++++++------------ src/backend/jit/llvm/llvmjit_expr.c | 7 +- src/backend/jit/llvm/llvmjit_types.c | 3 +- src/include/executor/execExpr.h | 3 +- 4 files changed, 136 insertions(+), 102 deletions(-) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index c2b76a5e5db..aee37cf50d5 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -2343,7 +2343,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) EEO_CASE(EEOP_AGG_PLAIN_TRANS_BATCH_ROWLOOP) { /* too complex for an inline implementation */ - ExecAggPlainTransBatch(state, op, econtext); + ExecAggPlainTransBatchRowloop(state, op, econtext); EEO_NEXT(); } @@ -2351,7 +2351,8 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) EEO_CASE(EEOP_AGG_PLAIN_TRANS_BATCH_DIRECT) { /* too complex for an inline implementation */ - ExecAggPlainTransBatch(state, op, econtext); + ExecAggPlainTransBatchDirect(state, op, econtext); + EEO_NEXT(); } @@ -6072,131 +6073,157 @@ ExecBuildBatchVector(ExprState *state, ExprEvalStep *op, ExprContext *econtext, bv->nrows = i; } -void -ExecAggPlainTransBatch(ExprState *state, ExprEvalStep *op, ExprContext *econtext) +static bool +ExecAggPlainTransBatchInitTrans(ExprState *state, ExprEvalStep *op, + TupleBatch *b) { AggState *aggstate = castNode(AggState, state->parent); AggStatePerTrans pertrans = op->d.agg_trans.pertrans; AggStatePerGroup pergroup = &aggstate->all_pergroups[op->d.agg_trans.setoff][op->d.agg_trans.transno]; BatchVectorSlice *bvs = op->d.agg_trans.bvs; + const BatchVector *bv = bvs->bv; + int batch_nrows = bvs ? bvs->bv->nrows : b->nvalid; + bool found = false; FunctionCallInfo fcinfo = pertrans->transfn_fcinfo; FmgrInfo *finfo = fcinfo->flinfo; - Datum newVal; - TupleBatch *batch = econtext->outer_batch; - int batch_nrows = bvs ? bvs->bv->nrows : batch->nvalid; - int start_row = 0; - if (finfo->fn_strict) + if (!finfo->fn_strict || bvs == NULL) + return false; + + for (int i = 0; i < batch_nrows; i++) { - if (pergroup->noTransValue && bvs) + for (int j = 0; j < bvs->nargs; j++) { - const BatchVector *bv = bvs->bv; - bool found = false; - - Assert(bv); - for (int i = 0; i < batch_nrows; i++) + if (!bv->nulls[bvs->argoffs[j]][i]) { - for (int j = 0; j < bvs->nargs; j++) + fcinfo->args[1].value = bv->cols[bvs->argoffs[j]][i]; + fcinfo->args[1].isnull = false; + if (j == bvs->nargs - 1) { - if (!bv->nulls[bvs->argoffs[j]][i]) - { - fcinfo->args[1].value = bv->cols[bvs->argoffs[j]][i]; - fcinfo->args[1].isnull = false; - if (j == bvs->nargs - 1) - { - found = true; - break; - } - } - } - if (found) + found = true; break; + } } - /* If transValue has not yet been initialized, do so now. */ - ExecAggInitGroup(aggstate, pertrans, pergroup, - op->d.agg_trans.aggcontext); - start_row = 1; } - else if (pergroup->transValueIsNull) + if (found) + break; + } + /* If transValue has not yet been initialized, do so now. */ + ExecAggInitGroup(aggstate, pertrans, pergroup, + op->d.agg_trans.aggcontext); + return true; +} + +void +ExecAggPlainTransBatchDirect(ExprState *state, ExprEvalStep *op, ExprContext *econtext) +{ + AggState *aggstate = castNode(AggState, state->parent); + AggStatePerTrans pertrans = op->d.agg_trans.pertrans; + AggStatePerGroup pergroup = + &aggstate->all_pergroups[op->d.agg_trans.setoff][op->d.agg_trans.transno]; + BatchVectorSlice *bvs = op->d.agg_trans.bvs; + FunctionCallInfo fcinfo = pertrans->transfn_fcinfo; + Datum newVal; + TupleBatch *b = econtext->outer_batch; + int batch_nrows = bvs ? bvs->bv->nrows : b->nvalid; + int start_row = 0; + void *save = fcinfo->flinfo->fn_extra; + AggBulkArgs ba = {batch_nrows, start_row}; + + if (pergroup->noTransValue) + { + if (ExecAggPlainTransBatchInitTrans(state, op, b)) + start_row = 1; + else if (pergroup->transValueIsNull && fcinfo->flinfo->fn_strict) return; } - switch (ExecEvalStepOp(state, op)) + if (bvs) { - case EEOP_AGG_PLAIN_TRANS_BATCH_ROWLOOP: - /* Loop rows, call the original transfn per element using vector cols. */ - for (int i = start_row; i < batch_nrows; i++) - { - bool hasnull = false; + const BatchVector *bv = bvs->bv; + + Assert(bv); + ba.nargs = bvs->nargs; + ba.argoffs = bvs->argoffs; + ba.args = bv->cols; + ba.isnull = bv->nulls; + ba.hasnull = bv->hasnull; + } + fcinfo->flinfo->fn_extra = &ba; + fcinfo->args[0].value = pergroup->transValue; + fcinfo->args[0].isnull = pergroup->transValueIsNull; + fcinfo->isnull = false; /* just in case transfn doesn't set it */ + newVal = FunctionCallInvoke(fcinfo); /* one call for the entire slice */ + if (!pertrans->transtypeByVal && + DatumGetPointer(newVal) != DatumGetPointer(pergroup->transValue)) + newVal = ExecAggCopyTransValue(aggstate, pertrans, + newVal, fcinfo->isnull, + pergroup->transValue, + pergroup->transValueIsNull); + pergroup->transValue = newVal; + pergroup->transValueIsNull = fcinfo->isnull; + fcinfo->flinfo->fn_extra = save; +} - /* Set up fcinfo args 1..m from column vectors at row i. */ - if (bvs) - { - const BatchVector *bv = bvs->bv; +void +ExecAggPlainTransBatchRowloop(ExprState *state, ExprEvalStep *op, ExprContext *econtext) +{ + AggState *aggstate = castNode(AggState, state->parent); + AggStatePerTrans pertrans = op->d.agg_trans.pertrans; + AggStatePerGroup pergroup = + &aggstate->all_pergroups[op->d.agg_trans.setoff][op->d.agg_trans.transno]; + BatchVectorSlice *bvs = op->d.agg_trans.bvs; + FunctionCallInfo fcinfo = pertrans->transfn_fcinfo; + FmgrInfo *finfo = fcinfo->flinfo; + Datum newVal; + TupleBatch *b = econtext->outer_batch; + int batch_nrows = bvs ? bvs->bv->nrows : b->nvalid; + int start_row = 0; - for (int j = 0; j < bvs->nargs; j++) - { - int16 argoff = bvs->argoffs[j]; + if (pergroup->noTransValue) + { + if (ExecAggPlainTransBatchInitTrans(state, op, b)) + start_row = 1; + else if (pergroup->transValueIsNull && fcinfo->flinfo->fn_strict) + return; + } - fcinfo->args[j+1].value = bv->cols[argoff][i]; - fcinfo->args[j+1].isnull = bv->nulls[argoff][i]; - if (!hasnull && bv->nulls[argoff][i]) - hasnull = true; - } - } - /* fcinfo->args[0] is the existing transition state */ - if (finfo->fn_strict && hasnull) - continue; - fcinfo->args[0].value = pergroup->transValue; - fcinfo->args[0].isnull = pergroup->transValueIsNull; - newVal = FunctionCallInvoke(fcinfo); - if (!pertrans->transtypeByVal && - DatumGetPointer(newVal) != DatumGetPointer(pergroup->transValue)) - newVal = ExecAggCopyTransValue(aggstate, pertrans, - newVal, fcinfo->isnull, - pergroup->transValue, - pergroup->transValueIsNull); - pergroup->transValue = newVal; - pergroup->transValueIsNull = fcinfo->isnull; - } - break; + /* Loop rows, call the original transfn per element using vector cols. */ + for (int i = start_row; i < batch_nrows; i++) + { + bool hasnull = false; - case EEOP_AGG_PLAIN_TRANS_BATCH_DIRECT: + /* Set up fcinfo args 1..m from column vectors at row i. */ + if (bvs) + { + const BatchVector *bv = bvs->bv; + + for (int j = 0; j < bvs->nargs; j++) { - void *save = fcinfo->flinfo->fn_extra; - AggBulkArgs ba = {batch_nrows, start_row}; + int16 argoff = bvs->argoffs[j]; - if (bvs) - { - const BatchVector *bv = bvs->bv; - - Assert(bv); - ba.nargs = bvs->nargs; - ba.argoffs = bvs->argoffs; - ba.args = bv->cols; - ba.isnull = bv->nulls; - ba.hasnull = bv->hasnull; - } - fcinfo->flinfo->fn_extra = &ba; - fcinfo->args[0].value = pergroup->transValue; - fcinfo->args[0].isnull = pergroup->transValueIsNull; - fcinfo->isnull = false; /* just in case transfn doesn't set it */ - newVal = FunctionCallInvoke(fcinfo); /* one call for the entire slice */ - if (!pertrans->transtypeByVal && - DatumGetPointer(newVal) != DatumGetPointer(pergroup->transValue)) - newVal = ExecAggCopyTransValue(aggstate, pertrans, - newVal, fcinfo->isnull, - pergroup->transValue, - pergroup->transValueIsNull); - pergroup->transValue = newVal; - pergroup->transValueIsNull = fcinfo->isnull; - fcinfo->flinfo->fn_extra = save; + fcinfo->args[j+1].value = bv->cols[argoff][i]; + fcinfo->args[j+1].isnull = bv->nulls[argoff][i]; + if (!hasnull && bv->nulls[argoff][i]) + hasnull = true; } - break; + } - default: - elog(ERROR, "invalid ExprEvalOp in ExecAggPlainTransBatch()"); + if (finfo->fn_strict && hasnull) + continue; + /* fcinfo->args[0] is the existing transition state */ + fcinfo->args[0].value = pergroup->transValue; + fcinfo->args[0].isnull = pergroup->transValueIsNull; + newVal = FunctionCallInvoke(fcinfo); + if (!pertrans->transtypeByVal && + DatumGetPointer(newVal) != DatumGetPointer(pergroup->transValue)) + newVal = ExecAggCopyTransValue(aggstate, pertrans, + newVal, fcinfo->isnull, + pergroup->transValue, + pergroup->transValueIsNull); + pergroup->transValue = newVal; + pergroup->transValueIsNull = fcinfo->isnull; } } diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index b97d5faebde..2d1c8259d1a 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -3027,8 +3027,13 @@ llvm_compile_expr(ExprState *state) break; case EEOP_AGG_PLAIN_TRANS_BATCH_DIRECT: + build_EvalXFunc(b, mod, "ExecAggPlainTransBatchDirect", + v_state, op, v_econtext); + LLVMBuildBr(b, opblocks[opno + 1]); + break; + case EEOP_AGG_PLAIN_TRANS_BATCH_ROWLOOP: - build_EvalXFunc(b, mod, "ExecAggPlainTransBatch", + build_EvalXFunc(b, mod, "ExecAggPlainTransBatchRowloop", v_state, op, v_econtext); LLVMBuildBr(b, opblocks[opno + 1]); break; diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c index f4f756e7cb5..2cf3a60be51 100644 --- a/src/backend/jit/llvm/llvmjit_types.c +++ b/src/backend/jit/llvm/llvmjit_types.c @@ -186,7 +186,8 @@ void *referenced_functions[] = ExecBuildInnerBatchVector, ExecBuildOuterBatchVector, ExecBuildScanBatchVector, - ExecAggPlainTransBatch, + ExecAggPlainTransBatchDirect, + ExecAggPlainTransBatchRowloop, ExecQualBatchInitMask, ExecQualBatchTerm, }; diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index f50936acaaa..a3314ffd0c9 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -993,7 +993,8 @@ extern void ExecBuildInnerBatchVector(ExprState *state, ExprEvalStep *op, ExprCo extern void ExecBuildOuterBatchVector(ExprState *state, ExprEvalStep *op, ExprContext *econtext); extern void ExecBuildScanBatchVector(ExprState *state, ExprEvalStep *op, ExprContext *econtext); -extern void ExecAggPlainTransBatch(ExprState *state, ExprEvalStep *op, ExprContext *econtext); +extern void ExecAggPlainTransBatchDirect(ExprState *state, ExprEvalStep *op, ExprContext *econtext); +extern void ExecAggPlainTransBatchRowloop(ExprState *state, ExprEvalStep *op, ExprContext *econtext); /* See ExecQualBatchTerm(). */ typedef enum BatchQualTermKind -- 2.47.3