database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-28 23:42:10 +03:00
Code Activity

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

Browse Source 
In ad0bda5d24 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
This commit is contained in:
Andres Freund
2019-09-09 05:21:30 -07:00
parent c6ce5f71b0
commit 3fb307bc4a
12 changed files with 709 additions and 277 deletions
Download Patch File Download Diff File Expand all files Collapse all files

273
src/test/isolation/expected/eval-plan-qual.out
View File

@ -258,6 +258,273 @@ accountid balance
checking 1050
savings 600
starting permutation: wnested2 c1 c2 read
s2: WARNING: upid: text checking = text checking: t
s2: WARNING: up: numeric 600 > numeric 200.0: t
s2: WARNING: lock_id: text checking = text checking: t
s2: WARNING: lock_bal: numeric 600 > numeric 200.0: t
s2: WARNING: 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: WARNING: upid: text checking = text checking: t
s2: WARNING: up: numeric 600 > numeric 200.0: t
s2: WARNING: lock_id: text checking = text checking: t
s2: WARNING: 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: WARNING: lock_id: text checking = text checking: t
s2: WARNING: lock_bal: numeric 400 > numeric 200.0: t
s2: WARNING: upid: text checking = text checking: t
s2: WARNING: up: numeric 400 > numeric 200.0: t
s2: WARNING: lock_id: text checking = text checking: t
s2: WARNING: lock_bal: numeric 600 > numeric 200.0: t
s2: WARNING: lock_id: text checking = text checking: t
s2: WARNING: lock_bal: numeric 400 > numeric 200.0: t
s2: WARNING: 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: WARNING: upid: text checking = text checking: t
s2: WARNING: up: numeric 600 > numeric 200.0: t
s2: WARNING: lock_id: text checking = text checking: t
s2: WARNING: 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: WARNING: lock_id: text checking = text checking: t
s2: WARNING: lock_bal: numeric 400 > numeric 200.0: t
s2: WARNING: upid: text checking = text checking: t
s2: WARNING: up: numeric 200 > numeric 200.0: f
s2: WARNING: 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: WARNING: upid: text checking = text checking: t
s2: WARNING: up: numeric 600 > numeric 200.0: t
s2: WARNING: lock_id: text checking = text checking: t
s2: WARNING: 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: WARNING: lock_id: text checking = text checking: t
s2: WARNING: lock_bal: numeric 200 > numeric 200.0: f
s2: WARNING: lock_id: text savings = text checking: f
s2: WARNING: 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: WARNING: upid: text checking = text checking: t
s2: WARNING: up: numeric 600 > numeric 200.0: t
s2: WARNING: lock_id: text checking = text checking: t
s2: WARNING: 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: WARNING: lock_id: text checking = text checking: t
s2: WARNING: lock_bal: numeric 200 > numeric 200.0: f
s2: WARNING: lock_id: text savings = text checking: f
s2: WARNING: 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: WARNING: upid: text checking = text checking: t
s2: WARNING: up: numeric 600 > numeric 200.0: t
s2: WARNING: lock_id: text checking = text checking: t
s2: WARNING: 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: WARNING: upid: text cds = text checking: f
s2: WARNING: 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: WARNING: upid: text checking = text checking: t
s2: WARNING: up: numeric 600 > numeric 200.0: t
s2: WARNING: lock_id: text checking = text checking: t
s2: WARNING: 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: WARNING: lock_id: text cds = text checking: f
s2: WARNING: lock_id: text savings = text checking: f
s2: WARNING: 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;

52
src/test/isolation/specs/eval-plan-qual.spec
View File

@ -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 WARNING '%: % % % % %: %', p_comment, pg_typeof(p_a), p_a, p_op, pg_typeof(p_b), p_b, r;
RETURN r;
END;$$;
}
teardown
@ -53,15 +63,20 @@ 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"
setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
setup { BEGIN ISOLATION LEVEL READ COMMITTED; SET client_min_messages = 'WARNING'; }
# wx1 then wx2 checks the basic case of re-fetching up-to-date values
step "wx1" { UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking' RETURNING balance; }
# 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" {
@ -167,7 +184,7 @@ step "simplepartupdate_noroute" {
session "s2"
setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
setup { BEGIN ISOLATION LEVEL READ COMMITTED; SET client_min_messages = 'WARNING'; }
step "wx2" { UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance; }
step "wy2" { UPDATE accounts SET balance = balance + 1000 WHERE accountid = 'checking' AND balance < 1000 RETURNING balance; }
step "d2" { DELETE FROM accounts WHERE accountid = 'checking'; }
@ -231,11 +248,25 @@ 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; }
session "s3"
setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
setup { BEGIN ISOLATION LEVEL READ COMMITTED; SET client_min_messages = 'WARNING'; }
step "read" { SELECT * FROM accounts ORDER BY accountid; }
step "read_ext" { SELECT * FROM accounts_ext ORDER BY accountid; }
step "read_a" { SELECT * FROM table_a ORDER BY id; }
@ -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"