diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2018-01-12 15:46:38 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2018-01-12 15:46:38 -0500 |
commit | 8b0e5e7e7cf0f205d93677b39b4fcbe60dce3b34 (patch) | |
tree | fa507a67382cecf010af979bccfb17455ed6232f | |
parent | 493cdc8ee503eeb2a48736a3c5c2a59d52d26838 (diff) | |
download | postgresql-8b0e5e7e7cf0f205d93677b39b4fcbe60dce3b34.tar.gz |
Avoid unnecessary failure in SELECT concurrent with ALTER NO INHERIT.
If a query against an inheritance tree runs concurrently with an ALTER
TABLE that's disinheriting one of the tree members, it's possible to get
a "could not find inherited attribute" error because after obtaining lock
on the removed member, make_inh_translation_list sees that its columns
have attinhcount=0 and decides they aren't the columns it's looking for.
An ideal fix, perhaps, would avoid including such a just-removed member
table in the query at all; but there seems no way to accomplish that
without adding expensive catalog rechecks or creating a likelihood of
deadlocks. Instead, let's just drop the check on attinhcount. In this
way, a query that's included a just-disinherited child will still
succeed, which is not a completely unreasonable behavior.
This problem has existed for a long time, so back-patch to all supported
branches. Also add an isolation test verifying related behaviors.
Patch by me; the new isolation test is based on Kyotaro Horiguchi's work.
Discussion: https://postgr.es/m/20170626.174612.23936762.horiguchi.kyotaro@lab.ntt.co.jp
-rw-r--r-- | src/backend/optimizer/prep/prepunion.c | 4 | ||||
-rw-r--r-- | src/test/isolation/expected/alter-table-4.out | 57 | ||||
-rw-r--r-- | src/test/isolation/isolation_schedule | 1 | ||||
-rw-r--r-- | src/test/isolation/specs/alter-table-4.spec | 37 |
4 files changed, 97 insertions, 2 deletions
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 64829404e6..0b9842baef 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -1486,7 +1486,7 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation, */ if (old_attno < newnatts && (att = new_tupdesc->attrs[old_attno]) != NULL && - !att->attisdropped && att->attinhcount != 0 && + !att->attisdropped && strcmp(attname, NameStr(att->attname)) == 0) new_attno = old_attno; else @@ -1494,7 +1494,7 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation, for (new_attno = 0; new_attno < newnatts; new_attno++) { att = new_tupdesc->attrs[new_attno]; - if (!att->attisdropped && att->attinhcount != 0 && + if (!att->attisdropped && strcmp(attname, NameStr(att->attname)) == 0) break; } diff --git a/src/test/isolation/expected/alter-table-4.out b/src/test/isolation/expected/alter-table-4.out new file mode 100644 index 0000000000..d2dac0be09 --- /dev/null +++ b/src/test/isolation/expected/alter-table-4.out @@ -0,0 +1,57 @@ +Parsed test spec with 2 sessions + +starting permutation: s1b s1delc1 s2sel s1c s2sel +step s1b: BEGIN; +step s1delc1: ALTER TABLE c1 NO INHERIT p; +step s2sel: SELECT SUM(a) FROM p; <waiting ...> +step s1c: COMMIT; +step s2sel: <... completed> +sum + +11 +step s2sel: SELECT SUM(a) FROM p; +sum + +1 + +starting permutation: s1b s1delc1 s1addc2 s2sel s1c s2sel +step s1b: BEGIN; +step s1delc1: ALTER TABLE c1 NO INHERIT p; +step s1addc2: ALTER TABLE c2 INHERIT p; +step s2sel: SELECT SUM(a) FROM p; <waiting ...> +step s1c: COMMIT; +step s2sel: <... completed> +sum + +11 +step s2sel: SELECT SUM(a) FROM p; +sum + +101 + +starting permutation: s1b s1dropc1 s2sel s1c s2sel +step s1b: BEGIN; +step s1dropc1: DROP TABLE c1; +step s2sel: SELECT SUM(a) FROM p; <waiting ...> +step s1c: COMMIT; +step s2sel: <... completed> +sum + +1 +step s2sel: SELECT SUM(a) FROM p; +sum + +1 + +starting permutation: s1b s1delc1 s1modc1a s2sel s1c s2sel +step s1b: BEGIN; +step s1delc1: ALTER TABLE c1 NO INHERIT p; +step s1modc1a: ALTER TABLE c1 ALTER COLUMN a TYPE float; +step s2sel: SELECT SUM(a) FROM p; <waiting ...> +step s1c: COMMIT; +step s2sel: <... completed> +error in steps s1c s2sel: ERROR: attribute "a" of relation "c1" does not match parent's type +step s2sel: SELECT SUM(a) FROM p; +sum + +1 diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index ade7362631..e9ca1fe93b 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -39,4 +39,5 @@ test: nowait-5 test: drop-index-concurrently-1 test: multiple-cic test: alter-table-1 +test: alter-table-4 test: timeouts diff --git a/src/test/isolation/specs/alter-table-4.spec b/src/test/isolation/specs/alter-table-4.spec new file mode 100644 index 0000000000..a9c1a93723 --- /dev/null +++ b/src/test/isolation/specs/alter-table-4.spec @@ -0,0 +1,37 @@ +# ALTER TABLE - Add and remove inheritance with concurrent reads + +setup +{ + CREATE TABLE p (a integer); + INSERT INTO p VALUES(1); + CREATE TABLE c1 () INHERITS (p); + INSERT INTO c1 VALUES(10); + CREATE TABLE c2 (a integer); + INSERT INTO c2 VALUES(100); +} + +teardown +{ + DROP TABLE IF EXISTS c1, c2, p; +} + +session "s1" +step "s1b" { BEGIN; } +step "s1delc1" { ALTER TABLE c1 NO INHERIT p; } +step "s1modc1a" { ALTER TABLE c1 ALTER COLUMN a TYPE float; } +step "s1addc2" { ALTER TABLE c2 INHERIT p; } +step "s1dropc1" { DROP TABLE c1; } +step "s1c" { COMMIT; } + +session "s2" +step "s2sel" { SELECT SUM(a) FROM p; } + +# NO INHERIT will not be visible to concurrent select, +# since we identify children before locking them +permutation "s1b" "s1delc1" "s2sel" "s1c" "s2sel" +# adding inheritance likewise is not seen if s1 commits after s2 locks p +permutation "s1b" "s1delc1" "s1addc2" "s2sel" "s1c" "s2sel" +# but we do cope with DROP on a child table +permutation "s1b" "s1dropc1" "s2sel" "s1c" "s2sel" +# this case currently results in an error; doesn't seem worth preventing +permutation "s1b" "s1delc1" "s1modc1a" "s2sel" "s1c" "s2sel" |