Reorder EPQ work, to fix rowmark related bugs and improve efficiency.

In ad0bda5d24ea I changed the EvalPlanQual machinery to store
substitution tuples in slot, instead of using plain HeapTuples. The
main motivation for that was that using HeapTuples will be inefficient
for future tableams.  But it turns out that that conversion was buggy
for non-locking rowmarks - the wrong tuple descriptor was used to
create the slot.

As a secondary issue 5db6df0c0 changed ExecLockRows() to begin EPQ
earlier, to allow to fetch the locked rows directly into the EPQ
slots, instead of having to copy tuples around. Unfortunately, as Tom
complained, that forces some expensive initialization to happen
earlier.

As a third issue, the test coverage for EPQ was clearly insufficient.

Fixing the first issue is unfortunately not trivial: Non-locked row
marks were fetched at the start of EPQ, and we don't have the type
information for the rowmarks available at that point. While we could
change that, it's not easy. It might be worthwhile to change that at
some point, but to fix this bug, it seems better to delay fetching
non-locking rowmarks when they're actually needed, rather than
eagerly. They're referenced at most once, and in cases where EPQ
fails, might never be referenced. Fetching them when needed also
increases locality a bit.

To be able to fetch rowmarks during execution, rather than
initialization, we need to be able to access the active EPQState, as
that contains necessary data. To do so move EPQ related data from
EState to EPQState, and, only for EStates creates as part of EPQ,
reference the associated EPQState from EState.

To fix the second issue, change EPQ initialization to allow use of
EvalPlanQualSlot() to be used before EvalPlanQualBegin() (but
obviously still requiring EvalPlanQualInit() to have been done).

As these changes made struct EState harder to understand, e.g. by
adding multiple EStates, significantly reorder the members, and add a
lot more comments.

Also add a few more EPQ tests, including one that fails for the first
issue above. More is needed.

Reported-By: yi huang
Author: Andres Freund
Reviewed-By: Tom Lane
Discussion:
    https://postgr.es/m/CAHU7rYZo_C4ULsAx_LAj8az9zqgrD8WDd4hTegDTMM1LMqrBsg@mail.gmail.com
    https://postgr.es/m/24530.1562686693@sss.pgh.pa.us
Backpatch: 12-, where the EPQ changes were introduced

2 files changed, 314 insertions, 5 deletions

diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out
index 5bf6ec1c273..65d3a5f0ae4 100644
--- a/src/test/isolation/expected/eval-plan-qual.out
+++ b/src/test/isolation/expected/eval-plan-qual.out
@@ -258,6 +258,273 @@ accountid      balance
 checking       1050           
 savings        600            
 
+starting permutation: wnested2 c1 c2 read
+s2: NOTICE:  upid: text checking = text checking: t
+s2: NOTICE:  up: numeric 600 > numeric 200.0: t
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 600 > numeric 200.0: t
+s2: NOTICE:  upid: text savings = text checking: f
+step wnested2: 
+    UPDATE accounts SET balance = balance - 1200
+    WHERE noisy_oper('upid', accountid, '=', 'checking')
+    AND noisy_oper('up', balance, '>', 200.0)
+    AND EXISTS (
+        SELECT accountid
+        FROM accounts_ext ae
+        WHERE noisy_oper('lock_id', ae.accountid, '=', accounts.accountid)
+            AND noisy_oper('lock_bal', ae.balance, '>', 200.0)
+        FOR UPDATE
+    );
+
+step c1: COMMIT;
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid      balance        
+
+checking       -600           
+savings        600            
+
+starting permutation: wx1 wxext1 wnested2 c1 c2 read
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+400            
+step wxext1: UPDATE accounts_ext SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+400            
+s2: NOTICE:  upid: text checking = text checking: t
+s2: NOTICE:  up: numeric 600 > numeric 200.0: t
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 600 > numeric 200.0: t
+step wnested2: 
+    UPDATE accounts SET balance = balance - 1200
+    WHERE noisy_oper('upid', accountid, '=', 'checking')
+    AND noisy_oper('up', balance, '>', 200.0)
+    AND EXISTS (
+        SELECT accountid
+        FROM accounts_ext ae
+        WHERE noisy_oper('lock_id', ae.accountid, '=', accounts.accountid)
+            AND noisy_oper('lock_bal', ae.balance, '>', 200.0)
+        FOR UPDATE
+    );
+ <waiting ...>
+step c1: COMMIT;
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 400 > numeric 200.0: t
+s2: NOTICE:  upid: text checking = text checking: t
+s2: NOTICE:  up: numeric 400 > numeric 200.0: t
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 600 > numeric 200.0: t
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 400 > numeric 200.0: t
+s2: NOTICE:  upid: text savings = text checking: f
+step wnested2: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid      balance        
+
+checking       -800           
+savings        600            
+
+starting permutation: wx1 wx1 wxext1 wnested2 c1 c2 read
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+400            
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+200            
+step wxext1: UPDATE accounts_ext SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+400            
+s2: NOTICE:  upid: text checking = text checking: t
+s2: NOTICE:  up: numeric 600 > numeric 200.0: t
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 600 > numeric 200.0: t
+step wnested2: 
+    UPDATE accounts SET balance = balance - 1200
+    WHERE noisy_oper('upid', accountid, '=', 'checking')
+    AND noisy_oper('up', balance, '>', 200.0)
+    AND EXISTS (
+        SELECT accountid
+        FROM accounts_ext ae
+        WHERE noisy_oper('lock_id', ae.accountid, '=', accounts.accountid)
+            AND noisy_oper('lock_bal', ae.balance, '>', 200.0)
+        FOR UPDATE
+    );
+ <waiting ...>
+step c1: COMMIT;
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 400 > numeric 200.0: t
+s2: NOTICE:  upid: text checking = text checking: t
+s2: NOTICE:  up: numeric 200 > numeric 200.0: f
+s2: NOTICE:  upid: text savings = text checking: f
+step wnested2: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid      balance        
+
+checking       200            
+savings        600            
+
+starting permutation: wx1 wx1 wxext1 wxext1 wnested2 c1 c2 read
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+400            
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+200            
+step wxext1: UPDATE accounts_ext SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+400            
+step wxext1: UPDATE accounts_ext SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+200            
+s2: NOTICE:  upid: text checking = text checking: t
+s2: NOTICE:  up: numeric 600 > numeric 200.0: t
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 600 > numeric 200.0: t
+step wnested2: 
+    UPDATE accounts SET balance = balance - 1200
+    WHERE noisy_oper('upid', accountid, '=', 'checking')
+    AND noisy_oper('up', balance, '>', 200.0)
+    AND EXISTS (
+        SELECT accountid
+        FROM accounts_ext ae
+        WHERE noisy_oper('lock_id', ae.accountid, '=', accounts.accountid)
+            AND noisy_oper('lock_bal', ae.balance, '>', 200.0)
+        FOR UPDATE
+    );
+ <waiting ...>
+step c1: COMMIT;
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 200 > numeric 200.0: f
+s2: NOTICE:  lock_id: text savings = text checking: f
+s2: NOTICE:  upid: text savings = text checking: f
+step wnested2: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid      balance        
+
+checking       200            
+savings        600            
+
+starting permutation: wx1 wxext1 wxext1 wnested2 c1 c2 read
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+400            
+step wxext1: UPDATE accounts_ext SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+400            
+step wxext1: UPDATE accounts_ext SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+200            
+s2: NOTICE:  upid: text checking = text checking: t
+s2: NOTICE:  up: numeric 600 > numeric 200.0: t
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 600 > numeric 200.0: t
+step wnested2: 
+    UPDATE accounts SET balance = balance - 1200
+    WHERE noisy_oper('upid', accountid, '=', 'checking')
+    AND noisy_oper('up', balance, '>', 200.0)
+    AND EXISTS (
+        SELECT accountid
+        FROM accounts_ext ae
+        WHERE noisy_oper('lock_id', ae.accountid, '=', accounts.accountid)
+            AND noisy_oper('lock_bal', ae.balance, '>', 200.0)
+        FOR UPDATE
+    );
+ <waiting ...>
+step c1: COMMIT;
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 200 > numeric 200.0: f
+s2: NOTICE:  lock_id: text savings = text checking: f
+s2: NOTICE:  upid: text savings = text checking: f
+step wnested2: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid      balance        
+
+checking       400            
+savings        600            
+
+starting permutation: wx1 tocds1 wnested2 c1 c2 read
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+400            
+step tocds1: UPDATE accounts SET accountid = 'cds' WHERE accountid = 'checking';
+s2: NOTICE:  upid: text checking = text checking: t
+s2: NOTICE:  up: numeric 600 > numeric 200.0: t
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 600 > numeric 200.0: t
+step wnested2: 
+    UPDATE accounts SET balance = balance - 1200
+    WHERE noisy_oper('upid', accountid, '=', 'checking')
+    AND noisy_oper('up', balance, '>', 200.0)
+    AND EXISTS (
+        SELECT accountid
+        FROM accounts_ext ae
+        WHERE noisy_oper('lock_id', ae.accountid, '=', accounts.accountid)
+            AND noisy_oper('lock_bal', ae.balance, '>', 200.0)
+        FOR UPDATE
+    );
+ <waiting ...>
+step c1: COMMIT;
+s2: NOTICE:  upid: text cds = text checking: f
+s2: NOTICE:  upid: text savings = text checking: f
+step wnested2: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid      balance        
+
+cds            400            
+savings        600            
+
+starting permutation: wx1 tocdsext1 wnested2 c1 c2 read
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
+balance        
+
+400            
+step tocdsext1: UPDATE accounts_ext SET accountid = 'cds' WHERE accountid = 'checking';
+s2: NOTICE:  upid: text checking = text checking: t
+s2: NOTICE:  up: numeric 600 > numeric 200.0: t
+s2: NOTICE:  lock_id: text checking = text checking: t
+s2: NOTICE:  lock_bal: numeric 600 > numeric 200.0: t
+step wnested2: 
+    UPDATE accounts SET balance = balance - 1200
+    WHERE noisy_oper('upid', accountid, '=', 'checking')
+    AND noisy_oper('up', balance, '>', 200.0)
+    AND EXISTS (
+        SELECT accountid
+        FROM accounts_ext ae
+        WHERE noisy_oper('lock_id', ae.accountid, '=', accounts.accountid)
+            AND noisy_oper('lock_bal', ae.balance, '>', 200.0)
+        FOR UPDATE
+    );
+ <waiting ...>
+step c1: COMMIT;
+s2: NOTICE:  lock_id: text cds = text checking: f
+s2: NOTICE:  lock_id: text savings = text checking: f
+s2: NOTICE:  upid: text savings = text checking: f
+step wnested2: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid      balance        
+
+checking       400            
+savings        600            
+
 starting permutation: wx1 updwcte c1 c2 read
 step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance;
 balance        
@@ -435,8 +702,10 @@ balance
 
 1050           
 step lockwithvalues: 
-	SELECT * FROM accounts a1, (values('checking'),('savings')) v(id)
-	  WHERE a1.accountid = v.id
+	-- Reference rowmark column that differs in type from targetlist at some attno.
+	-- See CAHU7rYZo_C4ULsAx_LAj8az9zqgrD8WDd4hTegDTMM1LMqrBsg@mail.gmail.com
+	SELECT a1.*, v.id FROM accounts a1, (values('checking'::text, 'nan'::text),('savings', 'nan')) v(id, notnumeric)
+	WHERE a1.accountid = v.id AND v.notnumeric != 'einszwei'
 	  FOR UPDATE OF a1;
  <waiting ...>
 step c2: COMMIT;
diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec
index f35a64ef63e..222195873ac 100644
--- a/src/test/isolation/specs/eval-plan-qual.spec
+++ b/src/test/isolation/specs/eval-plan-qual.spec
@@ -42,6 +42,16 @@ setup
  CREATE TABLE another_parttbl1 PARTITION OF another_parttbl FOR VALUES IN (1);
  CREATE TABLE another_parttbl2 PARTITION OF another_parttbl FOR VALUES IN (2);
  INSERT INTO another_parttbl VALUES (1, 1, 1);
+
+ CREATE FUNCTION noisy_oper(p_comment text, p_a anynonarray, p_op text, p_b anynonarray)
+ RETURNS bool LANGUAGE plpgsql AS $$
+ DECLARE
+  r bool;
+  BEGIN
+  EXECUTE format('SELECT $1 %s $2', p_op) INTO r USING p_a, p_b;
+  RAISE NOTICE '%: % % % % %: %', p_comment, pg_typeof(p_a), p_a, p_op, pg_typeof(p_b), p_b, r;
+  RETURN r;
+  END;$$;
 }
 
 teardown
@@ -53,6 +63,7 @@ teardown
  DROP TABLE table_a, table_b, jointest;
  DROP TABLE parttbl;
  DROP TABLE another_parttbl;
+ DROP FUNCTION noisy_oper(text, anynonarray, text, anynonarray)
 }
 
 session "s1"
@@ -62,6 +73,10 @@ step "wx1"	{ UPDATE accounts SET balance = balance - 200 WHERE accountid = 'chec
 # wy1 then wy2 checks the case where quals pass then fail
 step "wy1"	{ UPDATE accounts SET balance = balance + 500 WHERE accountid = 'checking' RETURNING balance; }
 
+step "wxext1"	{ UPDATE accounts_ext SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance; }
+step "tocds1"	{ UPDATE accounts SET accountid = 'cds' WHERE accountid = 'checking'; }
+step "tocdsext1" { UPDATE accounts_ext SET accountid = 'cds' WHERE accountid = 'checking'; }
+
 # d1 then wx1 checks that update can deal with the updated row vanishing
 # wx2 then d1 checks that the delete affects the updated row
 # wx2, wx2 then d1 checks that the delete checks the quals correctly (balance too high)
@@ -89,7 +104,7 @@ step "writep2"	{ UPDATE p SET b = -b WHERE a = 1 AND c = 0; }
 step "c1"	{ COMMIT; }
 step "r1"	{ ROLLBACK; }
 
-# these tests are meant to exercise EvalPlanQualFetchRowMarks,
+# these tests are meant to exercise EvalPlanQualFetchRowMark,
 # ie, handling non-locked tables in an EvalPlanQual recheck
 
 step "partiallock"	{
@@ -98,8 +113,10 @@ step "partiallock"	{
 	  FOR UPDATE OF a1;
 }
 step "lockwithvalues"	{
-	SELECT * FROM accounts a1, (values('checking'),('savings')) v(id)
-	  WHERE a1.accountid = v.id
+	-- Reference rowmark column that differs in type from targetlist at some attno.
+	-- See CAHU7rYZo_C4ULsAx_LAj8az9zqgrD8WDd4hTegDTMM1LMqrBsg@mail.gmail.com
+	SELECT a1.*, v.id FROM accounts a1, (values('checking'::text, 'nan'::text),('savings', 'nan')) v(id, notnumeric)
+	WHERE a1.accountid = v.id AND v.notnumeric != 'einszwei'
 	  FOR UPDATE OF a1;
 }
 step "partiallock_ext"	{
@@ -231,6 +248,20 @@ step "updwctefail"  { WITH doup AS (UPDATE accounts SET balance = balance + 1100
 step "delwcte"  { WITH doup AS (UPDATE accounts SET balance = balance + 1100 WHERE accountid = 'checking' RETURNING *) DELETE FROM accounts a USING doup RETURNING *; }
 step "delwctefail"  { WITH doup AS (UPDATE accounts SET balance = balance + 1100 WHERE accountid = 'checking' RETURNING *, update_checking(999)) DELETE FROM accounts a USING doup RETURNING *; }
 
+# Check that nested EPQ works correctly
+step "wnested2" {
+    UPDATE accounts SET balance = balance - 1200
+    WHERE noisy_oper('upid', accountid, '=', 'checking')
+    AND noisy_oper('up', balance, '>', 200.0)
+    AND EXISTS (
+        SELECT accountid
+        FROM accounts_ext ae
+        WHERE noisy_oper('lock_id', ae.accountid, '=', accounts.accountid)
+            AND noisy_oper('lock_bal', ae.balance, '>', 200.0)
+        FOR UPDATE
+    );
+}
+
 step "c2"	{ COMMIT; }
 step "r2"	{ ROLLBACK; }
 
@@ -282,6 +313,15 @@ permutation "wx2" "d2" "d1" "r2" "c1" "read"
 permutation "d1" "wx2" "c1" "c2" "read"
 permutation "d1" "wx2" "r1" "c2" "read"
 
+# Check that nested EPQ works correctly
+permutation "wnested2" "c1" "c2" "read"
+permutation "wx1" "wxext1" "wnested2" "c1" "c2" "read"
+permutation "wx1" "wx1" "wxext1" "wnested2" "c1" "c2" "read"
+permutation "wx1" "wx1" "wxext1" "wxext1" "wnested2" "c1" "c2" "read"
+permutation "wx1" "wxext1" "wxext1" "wnested2" "c1" "c2" "read"
+permutation "wx1" "tocds1" "wnested2" "c1" "c2" "read"
+permutation "wx1" "tocdsext1" "wnested2" "c1" "c2" "read"
+
 # test that an update to a self-modified row is ignored when
 # previously updated by the same cid
 permutation "wx1" "updwcte" "c1" "c2" "read"