diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index 743e7d636a..ab09691fb2 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -2992,12 +2992,18 @@ ExecEvalCase(CaseExprState *caseExpr, ExprContext *econtext, if (caseExpr->arg) { + Datum arg_value; bool arg_isNull; - econtext->caseValue_datum = ExecEvalExpr(caseExpr->arg, - econtext, - &arg_isNull, - NULL); + arg_value = ExecEvalExpr(caseExpr->arg, + econtext, + &arg_isNull, + NULL); + /* Since caseValue_datum may be read multiple times, force to R/O */ + econtext->caseValue_datum = + MakeExpandedObjectReadOnly(arg_value, + arg_isNull, + caseExpr->argtyplen); econtext->caseValue_isNull = arg_isNull; } @@ -4127,11 +4133,18 @@ ExecEvalCoerceToDomain(CoerceToDomainState *cstate, ExprContext *econtext, * nodes. We must save and restore prior setting of * econtext's domainValue fields, in case this node is * itself within a check expression for another domain. + * + * Also, if we are working with a read-write expanded + * datum, be sure that what we pass to CHECK expressions + * is a read-only pointer; else called functions might + * modify or even delete the expanded object. */ save_datum = econtext->domainValue_datum; save_isNull = econtext->domainValue_isNull; - econtext->domainValue_datum = result; + econtext->domainValue_datum = + MakeExpandedObjectReadOnly(result, *isNull, + cstate->constraint_ref->tcache->typlen); econtext->domainValue_isNull = *isNull; conResult = ExecEvalExpr(con->check_expr, @@ -4939,6 +4952,8 @@ ExecInitExpr(Expr *node, PlanState *parent) } cstate->args = outlist; cstate->defresult = ExecInitExpr(caseexpr->defresult, parent); + if (caseexpr->arg) + cstate->argtyplen = get_typlen(exprType((Node *) caseexpr->arg)); state = (ExprState *) cstate; } break; diff --git a/src/backend/utils/adt/domains.c b/src/backend/utils/adt/domains.c index 26bbbb5979..1cd80ae606 100644 --- a/src/backend/utils/adt/domains.c +++ b/src/backend/utils/adt/domains.c @@ -35,6 +35,7 @@ #include "executor/executor.h" #include "lib/stringinfo.h" #include "utils/builtins.h" +#include "utils/expandeddatum.h" #include "utils/lsyscache.h" #include "utils/syscache.h" #include "utils/typcache.h" @@ -166,9 +167,14 @@ domain_check_input(Datum value, bool isnull, DomainIOData *my_extra) * Set up value to be returned by CoerceToDomainValue * nodes. Unlike ExecEvalCoerceToDomain, this econtext * couldn't be shared with anything else, so no need to - * save and restore fields. + * save and restore fields. But we do need to protect the + * passed-in value against being changed by called + * functions. (It couldn't be a R/W expanded object for + * most uses, but that seems possible for domain_check().) */ - econtext->domainValue_datum = value; + econtext->domainValue_datum = + MakeExpandedObjectReadOnly(value, isnull, + my_extra->constraint_ref.tcache->typlen); econtext->domainValue_isNull = isnull; conResult = ExecEvalExprSwitchContext(con->check_expr, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 5c3b8683f5..cc0821bcac 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -889,6 +889,7 @@ typedef struct CaseExprState ExprState *arg; /* implicit equality comparison argument */ List *args; /* the arguments (list of WHEN clauses) */ ExprState *defresult; /* the default result (ELSE clause) */ + int16 argtyplen; /* if arg is provided, its typlen */ } CaseExprState; /* ---------------- diff --git a/src/include/utils/typcache.h b/src/include/utils/typcache.h index c72efcccb1..d187650ebe 100644 --- a/src/include/utils/typcache.h +++ b/src/include/utils/typcache.h @@ -131,9 +131,9 @@ typedef struct DomainConstraintRef { List *constraints; /* list of DomainConstraintState nodes */ MemoryContext refctx; /* context holding DomainConstraintRef */ + TypeCacheEntry *tcache; /* typcache entry for domain type */ /* Management data --- treat these fields as private to typcache.c */ - TypeCacheEntry *tcache; /* owning typcache entry */ DomainConstraintCache *dcc; /* current constraints, or NULL if none */ MemoryContextCallback callback; /* used to release refcount when done */ } DomainConstraintRef; diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out index 5f6aa16d31..09d5516fb5 100644 --- a/src/test/regress/expected/case.out +++ b/src/test/regress/expected/case.out @@ -338,6 +338,32 @@ SELECT CASE volfoo('bar') WHEN 'foo'::foodomain THEN 'is foo' ELSE 'is not foo' is not foo (1 row) +ROLLBACK; +-- Test multiple evaluation of a CASE arg that is a read/write object (#14472) +-- Wrap this in a single transaction so the transient '=' operator doesn't +-- cause problems in concurrent sessions +BEGIN; +CREATE DOMAIN arrdomain AS int[]; +CREATE FUNCTION make_ad(int,int) returns arrdomain as + 'declare x arrdomain; + begin + x := array[$1,$2]; + return x; + end' language plpgsql volatile; +CREATE FUNCTION ad_eq(arrdomain, arrdomain) returns boolean as + 'begin return array_eq($1, $2); end' language plpgsql; +CREATE OPERATOR = (procedure = ad_eq, + leftarg = arrdomain, rightarg = arrdomain); +SELECT CASE make_ad(1,2) + WHEN array[2,4]::arrdomain THEN 'wrong' + WHEN array[2,5]::arrdomain THEN 'still wrong' + WHEN array[1,2]::arrdomain THEN 'right' + END; + case +------- + right +(1 row) + ROLLBACK; -- -- Clean up diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index ac8dd91d03..79513e4598 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -5664,3 +5664,23 @@ end; $$; ERROR: value for domain plpgsql_domain violates check constraint "plpgsql_domain_check" CONTEXT: PL/pgSQL function inline_code_block line 4 at assignment +-- Test handling of expanded array passed to a domain constraint (bug #14472) +create function plpgsql_arr_domain_check(val int[]) returns boolean as $$ +begin return val[1] > 0; end +$$ language plpgsql immutable; +create domain plpgsql_arr_domain as int[] check(plpgsql_arr_domain_check(value)); +do $$ +declare v_test plpgsql_arr_domain; +begin + v_test := array[1]; + v_test := v_test || 2; +end; +$$; +do $$ +declare v_test plpgsql_arr_domain := array[1]; +begin + v_test := 0 || v_test; -- fail +end; +$$; +ERROR: value for domain plpgsql_arr_domain violates check constraint "plpgsql_arr_domain_check" +CONTEXT: PL/pgSQL function inline_code_block line 4 at assignment diff --git a/src/test/regress/sql/case.sql b/src/test/regress/sql/case.sql index c860fae258..a7ae7b4a9e 100644 --- a/src/test/regress/sql/case.sql +++ b/src/test/regress/sql/case.sql @@ -200,6 +200,34 @@ SELECT CASE volfoo('bar') WHEN 'foo'::foodomain THEN 'is foo' ELSE 'is not foo' ROLLBACK; +-- Test multiple evaluation of a CASE arg that is a read/write object (#14472) +-- Wrap this in a single transaction so the transient '=' operator doesn't +-- cause problems in concurrent sessions +BEGIN; + +CREATE DOMAIN arrdomain AS int[]; + +CREATE FUNCTION make_ad(int,int) returns arrdomain as + 'declare x arrdomain; + begin + x := array[$1,$2]; + return x; + end' language plpgsql volatile; + +CREATE FUNCTION ad_eq(arrdomain, arrdomain) returns boolean as + 'begin return array_eq($1, $2); end' language plpgsql; + +CREATE OPERATOR = (procedure = ad_eq, + leftarg = arrdomain, rightarg = arrdomain); + +SELECT CASE make_ad(1,2) + WHEN array[2,4]::arrdomain THEN 'wrong' + WHEN array[2,5]::arrdomain THEN 'still wrong' + WHEN array[1,2]::arrdomain THEN 'right' + END; + +ROLLBACK; + -- -- Clean up -- diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index e7445be189..877d3ad08e 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -4452,3 +4452,26 @@ begin v_test := 0; -- fail end; $$; + +-- Test handling of expanded array passed to a domain constraint (bug #14472) + +create function plpgsql_arr_domain_check(val int[]) returns boolean as $$ +begin return val[1] > 0; end +$$ language plpgsql immutable; + +create domain plpgsql_arr_domain as int[] check(plpgsql_arr_domain_check(value)); + +do $$ +declare v_test plpgsql_arr_domain; +begin + v_test := array[1]; + v_test := v_test || 2; +end; +$$; + +do $$ +declare v_test plpgsql_arr_domain := array[1]; +begin + v_test := 0 || v_test; -- fail +end; +$$;