Hello,
At Wed, 29 Nov 2017 14:04:01 +0900, Michael Paquier <michael.paqu...@gmail.com>
wrote in <cab7npqtt-mj4s8qoo_c9cafacv0j3vhgg4_kw2u1wxvyeyb...@mail.gmail.com>
> On Tue, Sep 19, 2017 at 3:04 PM, Kyotaro HORIGUCHI
> <horiguchi.kyot...@lab.ntt.co.jp> wrote:
> >> By the way, I will take a look at your patch when I come back from the
> >> vacation. Meanwhile, I noticed that it needs another rebase after
> >> 0a480502b092 [1].
>
> Moved to CF 2018-01.
Thank you. (I'll do that by myself from the next CF)
This is rebased patch and additional patch of isolation test for
this problem.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 4336b15d2c0d95e7044746aa5c3ae622712e41b3 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp>
Date: Tue, 12 Dec 2017 17:06:53 +0900
Subject: [PATCH 1/2] Add isolation test
---
src/test/isolation/expected/select-noinherit.out | 9 +++++++++
src/test/isolation/isolation_schedule | 1 +
src/test/isolation/specs/select-noinherit.spec | 23 +++++++++++++++++++++++
3 files changed, 33 insertions(+)
create mode 100644 src/test/isolation/expected/select-noinherit.out
create mode 100644 src/test/isolation/specs/select-noinherit.spec
diff --git a/src/test/isolation/expected/select-noinherit.out b/src/test/isolation/expected/select-noinherit.out
new file mode 100644
index 0000000..5885167
--- /dev/null
+++ b/src/test/isolation/expected/select-noinherit.out
@@ -0,0 +1,9 @@
+Parsed test spec with 2 sessions
+
+starting permutation: alt1 sel2 c1
+step alt1: ALTER TABLE c1 NO INHERIT p;
+step sel2: SELECT * FROM p; <waiting ...>
+step c1: COMMIT;
+step sel2: <... completed>
+a
+
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index e41b916..6e04ea4 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -63,3 +63,4 @@ test: async-notify
test: vacuum-reltuples
test: timeouts
test: vacuum-concurrent-drop
+test: select-noinherit
diff --git a/src/test/isolation/specs/select-noinherit.spec b/src/test/isolation/specs/select-noinherit.spec
new file mode 100644
index 0000000..31662a9
--- /dev/null
+++ b/src/test/isolation/specs/select-noinherit.spec
@@ -0,0 +1,23 @@
+# SELECT and ALTER TABLE NO INHERIT test
+#
+
+setup
+{
+ CREATE TABLE p (a integer);
+ CREATE TABLE c1 () INHERITS (p);
+}
+
+teardown
+{
+ DROP TABLE p CASCADE;
+}
+
+session "s1"
+setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
+step "alt1" { ALTER TABLE c1 NO INHERIT p; }
+step "c1" { COMMIT; }
+
+session "s2"
+step "sel2" { SELECT * FROM p; }
+
+permutation "alt1" "sel2" "c1"
--
2.9.2
>From c2793d9200948d693150a5bbeb3815e3b5404be2 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp>
Date: Tue, 12 Dec 2017 17:38:12 +0900
Subject: [PATCH 2/2] Lock parent on ALTER TABLE NO INHERIT
NO INHERIT doesn't modify the parent at all but lock is required to
avoid error caused when a concurrent access see a false child.
---
src/backend/commands/tablecmds.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d979ce2..a8d119f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13246,7 +13246,28 @@ RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid,
reltype = ((AlterObjectSchemaStmt *) stmt)->objectType;
else if (IsA(stmt, AlterTableStmt))
- reltype = ((AlterTableStmt *) stmt)->relkind;
+ {
+ AlterTableStmt *alterstmt = (AlterTableStmt *)stmt;
+ ListCell *lc;
+
+ reltype = alterstmt->relkind;
+
+ foreach (lc, alterstmt->cmds)
+ {
+ AlterTableCmd *cmd = lfirst_node(AlterTableCmd, lc);
+ Assert(IsA(cmd, AlterTableCmd));
+
+ /*
+ * Though NO INHERIT doesn't modify the parent, lock on the parent
+ * is necessary so that no concurrent expansion of inheritances
+ * sees a false child and ends with ERROR. But no need to ascend
+ * further.
+ */
+ if (cmd->subtype == AT_DropInherit)
+ RangeVarGetRelid((RangeVar *)cmd->def,
+ AccessExclusiveLock, false);
+ }
+ }
else
{
reltype = OBJECT_TABLE; /* placate compiler */
--
2.9.2
