Fix order of operations in ExecEvalFieldStoreDeForm().

If the given composite datum is toasted out-of-line,
DatumGetHeapTupleHeader will perform database accesses to detoast it.
That can invalidate the result of get_cached_rowtype, as documented
(perhaps not plainly enough) in that function's API spec; which leads
to strange errors or crashes when we try to use the TupleDesc to read
the tuple.  In short then, trying to update a field of a composite
column could fail intermittently if the overall column value is wide
enough to require toasting.

We can fix the bug at no cost by just changing the order of
operations, since we don't need the TupleDesc until after detoasting.
(Other callers of get_cached_rowtype appear to get this right already,
so there's only one bug.)

Note that the added regression test case reveals this bug reliably
only with debug_discard_caches/CLOBBER_CACHE_ALWAYS.

Per bug #17994 from Alexander Lakhin.  Sadly, this patch does not fix
the missing-values issue revealed in the bug discussion; we'll need
some more work to cover that.

Discussion: https://postgr.es/m/17994-5c7100b51b4790e9@postgresql.org

2 files changed, 14 insertions, 0 deletions

diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out
index 88cfad4b1ef..d30cd95085c 100644
--- a/src/test/regress/expected/rowtypes.out
+++ b/src/test/regress/expected/rowtypes.out
@@ -139,6 +139,15 @@ select (fn).first, substr((fn).last, 1, 20), length((fn).last) from people;
  Jim   | abcdefghijklabcdefgh | 1200000
 (2 rows)
 
+-- try an update on a toasted composite value, too
+update people set fn.first = 'Jack';
+select (fn).first, substr((fn).last, 1, 20), length((fn).last) from people;
+ first |        substr        | length  
+-------+----------------------+---------
+ Jack  | Blow                 |       4
+ Jack  | abcdefghijklabcdefgh | 1200000
+(2 rows)
+
 -- Test row comparison semantics.  Prior to PG 8.2 we did this in a totally
 -- non-spec-compliant way.
 select ROW(1,2) < ROW(1,3) as true;
diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql
index b869da269c7..82878b6fb29 100644
--- a/src/test/regress/sql/rowtypes.sql
+++ b/src/test/regress/sql/rowtypes.sql
@@ -76,6 +76,11 @@ insert into people select ('Jim', f1, null)::fullname, current_date from pp;
 
 select (fn).first, substr((fn).last, 1, 20), length((fn).last) from people;
 
+-- try an update on a toasted composite value, too
+update people set fn.first = 'Jack';
+
+select (fn).first, substr((fn).last, 1, 20), length((fn).last) from people;
+
 -- Test row comparison semantics.  Prior to PG 8.2 we did this in a totally
 -- non-spec-compliant way.