Fix handling of expanded objects in CoerceToDomain and CASE execution.

When the input value to a CoerceToDomain expression node is a read-write
expanded datum, we should pass a read-only pointer to any domain CHECK
expressions and then return the original read-write pointer as the
expression result.  Previously we were blindly passing the same pointer to
all the consumers of the value, making it possible for a function in CHECK
to modify or even delete the expanded value.  (Since a plpgsql function
will absorb a passed-in read-write expanded array as a local variable
value, it will in fact delete the value on exit.)

A similar hazard of passing the same read-write pointer to multiple
consumers exists in domain_check() and in ExecEvalCase, so fix those too.

The fix requires adding MakeExpandedObjectReadOnly calls at the appropriate
places, which is simple enough except that we need to get the data type's
typlen from somewhere.  For the domain cases, solve this by redefining
DomainConstraintRef.tcache as okay for callers to access; there wasn't any
reason for the original convention against that, other than not wanting the
API of typcache.c to be any wider than it had to be.  For CASE, there's
no good solution except to add a syscache lookup during executor start.

Per bug #14472 from Marcos Castedo.  Back-patch to 9.5 where expanded
values were introduced.

Discussion: https://postgr.es/m/15225.1482431619@sss.pgh.pa.us

4 files changed, 97 insertions, 0 deletions

diff --git a/src/test/regress/expected/case.out b/src/test/regress/expected/case.out
index 5f6aa16d319..09d5516fb5f 100644
--- a/src/test/regress/expected/case.out
+++ b/src/test/regress/expected/case.out
@@ -339,6 +339,32 @@ SELECT CASE volfoo('bar') WHEN 'foo'::foodomain THEN 'is foo' ELSE '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 ac8dd91d03a..79513e4598b 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 c860fae258a..a7ae7b4a9eb 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 e7445be1896..877d3ad08ef 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;
+$$;