diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index ab89ad9afbf..1e3b93a69d8 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1188,6 +1188,14 @@ ExecInitExprRec(Expr *node, ExprState *state, op->args, op->opfuncid, op->inputcollid, state); + /* + * If first argument is of varlena type, we'll need to ensure + * that the value passed to the comparison function is a + * read-only pointer. + */ + scratch.d.func.make_ro = + (get_typlen(exprType((Node *) linitial(op->args))) == -1); + /* * Change opcode of call instruction to EEOP_NULLIF. * diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index aa68c115ba9..a55ad772566 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -1295,12 +1295,24 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) * The arguments are already evaluated into fcinfo->args. */ FunctionCallInfo fcinfo = op->d.func.fcinfo_data; + Datum save_arg0 = fcinfo->args[0].value; /* if either argument is NULL they can't be equal */ if (!fcinfo->args[0].isnull && !fcinfo->args[1].isnull) { Datum result; + /* + * If first argument is of varlena type, it might be an + * expanded datum. We need to ensure that the value passed to + * the comparison function is a read-only pointer. However, + * if we end by returning the first argument, that will be the + * original read-write pointer if it was read-write. + */ + if (op->d.func.make_ro) + fcinfo->args[0].value = + MakeExpandedObjectReadOnlyInternal(save_arg0); + fcinfo->isnull = false; result = op->d.func.fn_addr(fcinfo); @@ -1315,7 +1327,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) } /* Arguments aren't equal, so return the first one */ - *op->resvalue = fcinfo->args[0].value; + *op->resvalue = save_arg0; *op->resnull = fcinfo->args[0].isnull; EEO_NEXT(); diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index 306aea82d3b..b9fdd444a25 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -1562,6 +1562,9 @@ llvm_compile_expr(ExprState *state) v_fcinfo = l_ptr_const(fcinfo, l_ptr(StructFunctionCallInfoData)); + /* save original arg[0] */ + v_arg0 = l_funcvalue(b, v_fcinfo, 0); + /* if either argument is NULL they can't be equal */ v_argnull0 = l_funcnull(b, v_fcinfo, 0); v_argnull1 = l_funcnull(b, v_fcinfo, 1); @@ -1578,7 +1581,6 @@ llvm_compile_expr(ExprState *state) /* one (or both) of the arguments are null, return arg[0] */ LLVMPositionBuilderAtEnd(b, b_hasnull); - v_arg0 = l_funcvalue(b, v_fcinfo, 0); LLVMBuildStore(b, v_argnull0, v_resnullp); LLVMBuildStore(b, v_arg0, v_resvaluep); LLVMBuildBr(b, opblocks[opno + 1]); @@ -1586,12 +1588,35 @@ llvm_compile_expr(ExprState *state) /* build block to invoke function and check result */ LLVMPositionBuilderAtEnd(b, b_nonull); + /* + * If first argument is of varlena type, it might be an + * expanded datum. We need to ensure that the value + * passed to the comparison function is a read-only + * pointer. However, if we end by returning the first + * argument, that will be the original read-write pointer + * if it was read-write. + */ + if (op->d.func.make_ro) + { + LLVMValueRef v_params[1]; + LLVMValueRef v_arg0_ro; + + v_params[0] = v_arg0; + v_arg0_ro = + l_call(b, + llvm_pg_var_func_type("MakeExpandedObjectReadOnlyInternal"), + llvm_pg_func(mod, "MakeExpandedObjectReadOnlyInternal"), + v_params, lengthof(v_params), ""); + LLVMBuildStore(b, v_arg0_ro, + l_funcvaluep(b, v_fcinfo, 0)); + } + v_retval = BuildV1Call(context, b, mod, fcinfo, &v_fcinfo_isnull); /* - * If result not null, and arguments are equal return null - * (same result as if there'd been NULLs, hence reuse - * b_hasnull). + * If result not null and arguments are equal return null, + * else return arg[0] (same result as if there'd been + * NULLs, hence reuse b_hasnull). */ v_argsequal = LLVMBuildAnd(b, LLVMBuildICmp(b, LLVMIntEQ, diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index d983a9a7fed..ef1fa37716d 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -356,6 +356,7 @@ typedef struct ExprEvalStep /* faster to access without additional indirection: */ PGFunction fn_addr; /* actual call address */ int nargs; /* number of arguments */ + bool make_ro; /* make arg0 R/O (used only for NULLIF) */ } func; /* for EEOP_BOOL_*_STEP */ diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out index f5136c17abb..efee7fc4317 100644 --- a/src/test/regress/expected/case.out +++ b/src/test/regress/expected/case.out @@ -397,6 +397,14 @@ SELECT CASE make_ad(1,2) right (1 row) +-- While we're here, also test handling of a NULLIF arg that is a read/write +-- object (bug #18722) +SELECT NULLIF(make_ad(1,2), array[2,3]::arrdomain); + nullif +-------- + {1,2} +(1 row) + ROLLBACK; -- Test interaction of CASE with ArrayCoerceExpr (bug #15471) BEGIN; diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql index 83fe43be6b8..388d4c6f528 100644 --- a/src/test/regress/sql/case.sql +++ b/src/test/regress/sql/case.sql @@ -242,6 +242,11 @@ SELECT CASE make_ad(1,2) WHEN array[1,2]::arrdomain THEN 'right' END; +-- While we're here, also test handling of a NULLIF arg that is a read/write +-- object (bug #18722) + +SELECT NULLIF(make_ad(1,2), array[2,3]::arrdomain); + ROLLBACK; -- Test interaction of CASE with ArrayCoerceExpr (bug #15471)