From 0d597c7bb5e6df237a046528922709605bac0cea Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 29 Jan 2025 21:12:34 -0500 Subject: [PATCH v1 7/8] Remove dead code for executor support of CaseTestExpr. If a CaseTestExpr manages to get through the planner, we'll now get "unrecognized node type", which is good. I also took the opportunity to rename ExprEvalStep's "casetest" variant to "domaintest", since it's now only used for that. --- src/backend/executor/execExpr.c | 38 +---------- src/backend/executor/execExprInterp.c | 84 +++-------------------- src/backend/executor/execUtils.c | 6 -- src/backend/jit/llvm/llvmjit_expr.c | 98 ++++++--------------------- src/include/executor/execExpr.h | 7 +- src/include/nodes/execnodes.h | 17 ++--- 6 files changed, 39 insertions(+), 211 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 6d2b9179b2..8c8e4db419 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1823,25 +1823,11 @@ ExecInitExprRec(Expr *node, ExprState *state, foreach(lc, caseExpr->args) { CaseWhen *when = (CaseWhen *) lfirst(lc); - Datum *save_innermost_caseval; - bool *save_innermost_casenull; int whenstep; - /* - * XXX innermost_caseval/casenull will go away, but for - * now just make them NULL. - */ - save_innermost_caseval = state->innermost_caseval; - save_innermost_casenull = state->innermost_casenull; - state->innermost_caseval = NULL; - state->innermost_casenull = NULL; - /* evaluate condition into CASE's result variables */ ExecInitExprRec(when->expr, state, resv, resnull); - state->innermost_caseval = save_innermost_caseval; - state->innermost_casenull = save_innermost_casenull; - /* If WHEN result isn't true, jump to next CASE arm */ scratch.opcode = EEOP_JUMP_IF_NOT_TRUE; scratch.d.jump.jumpdone = -1; /* computed later */ @@ -1893,25 +1879,6 @@ ExecInitExprRec(Expr *node, ExprState *state, break; } - case T_CaseTestExpr: - { - /* - * Read from location identified by innermost_caseval. Note - * that innermost_caseval could be NULL, if this node isn't - * actually within a CaseExpr, ArrayCoerceExpr, etc structure. - * That can happen because some parts of the system abuse - * CaseTestExpr to cause a read of a value externally supplied - * in econtext->caseValue_datum. We'll take care of that - * scenario at runtime. - */ - scratch.opcode = EEOP_CASE_TESTVAL; - scratch.d.casetest.value = state->innermost_caseval; - scratch.d.casetest.isnull = state->innermost_casenull; - - ExprEvalPushStep(state, &scratch); - break; - } - case T_ArrayExpr: { ArrayExpr *arrayexpr = (ArrayExpr *) node; @@ -2616,9 +2583,8 @@ ExecInitExprRec(Expr *node, ExprState *state, * scenario at runtime. */ scratch.opcode = EEOP_DOMAIN_TESTVAL; - /* we share instruction union variant with case testval */ - scratch.d.casetest.value = state->innermost_domainval; - scratch.d.casetest.isnull = state->innermost_domainnull; + scratch.d.domaintest.value = state->innermost_domainval; + scratch.d.domaintest.isnull = state->innermost_domainnull; ExprEvalPushStep(state, &scratch); break; diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index fa3e76eccd..57060686b1 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -160,7 +160,6 @@ static Datum ExecJustScanVar(ExprState *state, ExprContext *econtext, bool *isnu static Datum ExecJustAssignInnerVar(ExprState *state, ExprContext *econtext, bool *isnull); static Datum ExecJustAssignOuterVar(ExprState *state, ExprContext *econtext, bool *isnull); static Datum ExecJustAssignScanVar(ExprState *state, ExprContext *econtext, bool *isnull); -static Datum ExecJustApplyFuncToCase(ExprState *state, ExprContext *econtext, bool *isnull); static Datum ExecJustConst(ExprState *state, ExprContext *econtext, bool *isnull); static Datum ExecJustInnerVarVirt(ExprState *state, ExprContext *econtext, bool *isnull); static Datum ExecJustOuterVarVirt(ExprState *state, ExprContext *econtext, bool *isnull); @@ -364,13 +363,6 @@ ExecReadyInterpretedExpr(ExprState *state) state->evalfunc_private = ExecJustAssignScanVar; return; } - else if (step0 == EEOP_CASE_TESTVAL && - step1 == EEOP_FUNCEXPR_STRICT && - state->steps[0].d.casetest.value) - { - state->evalfunc_private = ExecJustApplyFuncToCase; - return; - } else if (step0 == EEOP_INNER_VAR && step1 == EEOP_HASHDATUM_FIRST) { @@ -525,7 +517,6 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) &&CASE_EEOP_PARAM_CALLBACK, &&CASE_EEOP_PARAM_SET_EXEC, &&CASE_EEOP_PARAM_SET_EXPR, - &&CASE_EEOP_CASE_TESTVAL, &&CASE_EEOP_MAKE_READONLY, &&CASE_EEOP_IOCOERCE, &&CASE_EEOP_IOCOERCE_SAFE, @@ -1288,40 +1279,21 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) EEO_NEXT(); } - EEO_CASE(EEOP_CASE_TESTVAL) + EEO_CASE(EEOP_DOMAIN_TESTVAL) { /* * Normally upper parts of the expression tree have setup the * values to be returned here, but some parts of the system - * currently misuse {caseValue,domainValue}_{datum,isNull} to set - * run-time data. So if no values have been set-up, use - * ExprContext's. This isn't pretty, but also not *that* ugly, - * and this is unlikely to be performance sensitive enough to - * worry about an extra branch. + * currently misuse domainValue_{datum,isNull} to set run-time + * data. So if no values have been set-up, use ExprContext's. + * This isn't pretty, but also not *that* ugly, and this is + * unlikely to be performance sensitive enough to worry about an + * extra branch. */ - if (op->d.casetest.value) + if (op->d.domaintest.value) { - *op->resvalue = *op->d.casetest.value; - *op->resnull = *op->d.casetest.isnull; - } - else - { - *op->resvalue = econtext->caseValue_datum; - *op->resnull = econtext->caseValue_isNull; - } - - EEO_NEXT(); - } - - EEO_CASE(EEOP_DOMAIN_TESTVAL) - { - /* - * See EEOP_CASE_TESTVAL comment. - */ - if (op->d.casetest.value) - { - *op->resvalue = *op->d.casetest.value; - *op->resnull = *op->d.casetest.isnull; + *op->resvalue = *op->d.domaintest.value; + *op->resnull = *op->d.domaintest.isnull; } else { @@ -2569,44 +2541,6 @@ ExecJustAssignScanVar(ExprState *state, ExprContext *econtext, bool *isnull) return ExecJustAssignVarImpl(state, econtext->ecxt_scantuple, isnull); } -/* Evaluate CASE_TESTVAL and apply a strict function to it */ -static Datum -ExecJustApplyFuncToCase(ExprState *state, ExprContext *econtext, bool *isnull) -{ - ExprEvalStep *op = &state->steps[0]; - FunctionCallInfo fcinfo; - NullableDatum *args; - int nargs; - Datum d; - - /* - * XXX with some redesign of the CaseTestExpr mechanism, maybe we could - * get rid of this data shuffling? - */ - *op->resvalue = *op->d.casetest.value; - *op->resnull = *op->d.casetest.isnull; - - op++; - - nargs = op->d.func.nargs; - fcinfo = op->d.func.fcinfo_data; - args = fcinfo->args; - - /* strict function, so check for NULL args */ - for (int argno = 0; argno < nargs; argno++) - { - if (args[argno].isnull) - { - *isnull = true; - return (Datum) 0; - } - } - fcinfo->isnull = false; - d = op->d.func.fn_addr(fcinfo); - *isnull = fcinfo->isnull; - return d; -} - /* Simple Const expression */ static Datum ExecJustConst(ExprState *state, ExprContext *econtext, bool *isnull) diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 9a236c3789..fd541c23ec 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -271,9 +271,6 @@ CreateExprContextInternal(EState *estate, Size minContextSize, econtext->ecxt_aggvalues = NULL; econtext->ecxt_aggnulls = NULL; - econtext->caseValue_datum = (Datum) 0; - econtext->caseValue_isNull = true; - econtext->domainValue_datum = (Datum) 0; econtext->domainValue_isNull = true; @@ -388,9 +385,6 @@ CreateStandaloneExprContext(void) econtext->ecxt_aggvalues = NULL; econtext->ecxt_aggnulls = NULL; - econtext->caseValue_datum = (Datum) 0; - econtext->caseValue_isNull = true; - econtext->domainValue_datum = (Datum) 0; econtext->domainValue_isNull = true; diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index 1b59c30b6f..7c73cca28a 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -1275,60 +1275,6 @@ llvm_compile_expr(ExprState *state) break; } - case EEOP_CASE_TESTVAL: - { - LLVMBasicBlockRef b_avail, - b_notavail; - LLVMValueRef v_casevaluep, - v_casevalue; - LLVMValueRef v_casenullp, - v_casenull; - LLVMValueRef v_casevaluenull; - - b_avail = l_bb_before_v(opblocks[opno + 1], - "op.%d.avail", opno); - b_notavail = l_bb_before_v(opblocks[opno + 1], - "op.%d.notavail", opno); - - v_casevaluep = l_ptr_const(op->d.casetest.value, - l_ptr(TypeSizeT)); - v_casenullp = l_ptr_const(op->d.casetest.isnull, - l_ptr(TypeStorageBool)); - - v_casevaluenull = - LLVMBuildICmp(b, LLVMIntEQ, - LLVMBuildPtrToInt(b, v_casevaluep, - TypeSizeT, ""), - l_sizet_const(0), ""); - LLVMBuildCondBr(b, v_casevaluenull, b_notavail, b_avail); - - /* if casetest != NULL */ - LLVMPositionBuilderAtEnd(b, b_avail); - v_casevalue = l_load(b, TypeSizeT, v_casevaluep, ""); - v_casenull = l_load(b, TypeStorageBool, v_casenullp, ""); - LLVMBuildStore(b, v_casevalue, v_resvaluep); - LLVMBuildStore(b, v_casenull, v_resnullp); - LLVMBuildBr(b, opblocks[opno + 1]); - - /* if casetest == NULL */ - LLVMPositionBuilderAtEnd(b, b_notavail); - v_casevalue = - l_load_struct_gep(b, - StructExprContext, - v_econtext, - FIELDNO_EXPRCONTEXT_CASEDATUM, ""); - v_casenull = - l_load_struct_gep(b, - StructExprContext, - v_econtext, - FIELDNO_EXPRCONTEXT_CASENULL, ""); - LLVMBuildStore(b, v_casevalue, v_resvaluep); - LLVMBuildStore(b, v_casenull, v_resnullp); - - LLVMBuildBr(b, opblocks[opno + 1]); - break; - } - case EEOP_MAKE_READONLY: { LLVMBasicBlockRef b_notnull; @@ -1972,55 +1918,55 @@ llvm_compile_expr(ExprState *state) { LLVMBasicBlockRef b_avail, b_notavail; - LLVMValueRef v_casevaluep, - v_casevalue; - LLVMValueRef v_casenullp, - v_casenull; - LLVMValueRef v_casevaluenull; + LLVMValueRef v_domainvaluep, + v_domainvalue; + LLVMValueRef v_domainnullp, + v_domainnull; + LLVMValueRef v_domainvaluenull; b_avail = l_bb_before_v(opblocks[opno + 1], "op.%d.avail", opno); b_notavail = l_bb_before_v(opblocks[opno + 1], "op.%d.notavail", opno); - v_casevaluep = l_ptr_const(op->d.casetest.value, - l_ptr(TypeSizeT)); - v_casenullp = l_ptr_const(op->d.casetest.isnull, - l_ptr(TypeStorageBool)); + v_domainvaluep = l_ptr_const(op->d.domaintest.value, + l_ptr(TypeSizeT)); + v_domainnullp = l_ptr_const(op->d.domaintest.isnull, + l_ptr(TypeStorageBool)); - v_casevaluenull = + v_domainvaluenull = LLVMBuildICmp(b, LLVMIntEQ, - LLVMBuildPtrToInt(b, v_casevaluep, + LLVMBuildPtrToInt(b, v_domainvaluep, TypeSizeT, ""), l_sizet_const(0), ""); LLVMBuildCondBr(b, - v_casevaluenull, + v_domainvaluenull, b_notavail, b_avail); - /* if casetest != NULL */ + /* if domaintest != NULL */ LLVMPositionBuilderAtEnd(b, b_avail); - v_casevalue = l_load(b, TypeSizeT, v_casevaluep, ""); - v_casenull = l_load(b, TypeStorageBool, v_casenullp, ""); - LLVMBuildStore(b, v_casevalue, v_resvaluep); - LLVMBuildStore(b, v_casenull, v_resnullp); + v_domainvalue = l_load(b, TypeSizeT, v_domainvaluep, ""); + v_domainnull = l_load(b, TypeStorageBool, v_domainnullp, ""); + LLVMBuildStore(b, v_domainvalue, v_resvaluep); + LLVMBuildStore(b, v_domainnull, v_resnullp); LLVMBuildBr(b, opblocks[opno + 1]); - /* if casetest == NULL */ + /* if domaintest == NULL */ LLVMPositionBuilderAtEnd(b, b_notavail); - v_casevalue = + v_domainvalue = l_load_struct_gep(b, StructExprContext, v_econtext, FIELDNO_EXPRCONTEXT_DOMAINDATUM, ""); - v_casenull = + v_domainnull = l_load_struct_gep(b, StructExprContext, v_econtext, FIELDNO_EXPRCONTEXT_DOMAINNULL, ""); - LLVMBuildStore(b, v_casevalue, v_resvaluep); - LLVMBuildStore(b, v_casenull, v_resnullp); + LLVMBuildStore(b, v_domainvalue, v_resvaluep); + LLVMBuildStore(b, v_domainnull, v_resnullp); LLVMBuildBr(b, opblocks[opno + 1]); break; diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index e03045de92..5e5623b11b 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -174,9 +174,6 @@ typedef enum ExprEvalOp EEOP_PARAM_SET_EXEC, EEOP_PARAM_SET_EXPR, - /* return CaseTestExpr value */ - EEOP_CASE_TESTVAL, - /* apply MakeExpandedObjectReadOnly() to target value */ EEOP_MAKE_READONLY, @@ -438,12 +435,12 @@ typedef struct ExprEvalStep Oid paramtype; /* OID of parameter's datatype */ } cparam; - /* for EEOP_CASE_TESTVAL/DOMAIN_TESTVAL */ + /* for EEOP_DOMAIN_TESTVAL */ struct { Datum *value; /* value to return */ bool *isnull; - } casetest; + } domaintest; /* for EEOP_MAKE_READONLY */ struct diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 366534c0d0..866b236ae4 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -134,9 +134,6 @@ typedef struct ExprState struct PlanState *parent; /* parent PlanState node, if any */ ParamListInfo ext_params; /* for compiling PARAM_EXTERN nodes */ - Datum *innermost_caseval; - bool *innermost_casenull; - Datum *innermost_domainval; bool *innermost_domainnull; @@ -289,22 +286,16 @@ typedef struct ExprContext #define FIELDNO_EXPRCONTEXT_AGGNULLS 11 bool *ecxt_aggnulls; /* null flags for aggs/windowfuncs */ - /* Value to substitute for CaseTestExpr nodes in expression */ -#define FIELDNO_EXPRCONTEXT_CASEDATUM 12 - Datum caseValue_datum; -#define FIELDNO_EXPRCONTEXT_CASENULL 13 - bool caseValue_isNull; - /* Value to substitute for CoerceToDomainValue nodes in expression */ -#define FIELDNO_EXPRCONTEXT_DOMAINDATUM 14 +#define FIELDNO_EXPRCONTEXT_DOMAINDATUM 12 Datum domainValue_datum; -#define FIELDNO_EXPRCONTEXT_DOMAINNULL 15 +#define FIELDNO_EXPRCONTEXT_DOMAINNULL 13 bool domainValue_isNull; /* Tuples that OLD/NEW Var nodes in RETURNING may refer to */ -#define FIELDNO_EXPRCONTEXT_OLDTUPLE 16 +#define FIELDNO_EXPRCONTEXT_OLDTUPLE 14 TupleTableSlot *ecxt_oldtuple; -#define FIELDNO_EXPRCONTEXT_NEWTUPLE 17 +#define FIELDNO_EXPRCONTEXT_NEWTUPLE 15 TupleTableSlot *ecxt_newtuple; /* Link to containing EState (NULL if a standalone ExprContext) */ -- 2.43.5