From de6fb197de32b6c50b4c07c4917075e957cd2cfb Mon Sep 17 00:00:00 2001
From: Nikita Pettik <korablev@tarantool.org>
Date: Tue, 19 Mar 2019 22:23:37 +0300
Subject: [PATCH] sql: make spaces without PK illegal in queries
Our SQL codebase was inherited from SQLite, where each table must have
at least one index - primary (if no explicit PK declared, one based on
rowid is implicitly created). In Tarantool spaces can exists without
indexes. The only existing restriction is that they can't contain any
data. Hence, even very basic queries fail with assertion/seagfault if
they are applied to spaces with no indexes. Situation when space turns
out to remain without PK is quite common due to the absence of
transactional DDL: for instance, space drop procedure consists of
several steps including dropping all indexes; space itself is dropped at
the very end. Thus, if a sequence of queries is interrupted by drop
space procedure and one is not finished, the rest of queries will
operate on space with no indexes.
As a solution to this problem (at least temporary) now we disallow
query processing involving spaces without PK except for views.
Closes #3780
---
src/box/sql/delete.c | 6 ++++
test/sql/errinj.result | 48 +++++++++++++++++++++++++++
test/sql/errinj.test.lua | 22 +++++++++++++
test/sql/no-pk-space.result | 61 +++++++++++++++++++++++++++++++++++
test/sql/no-pk-space.test.lua | 24 ++++++++++++++
5 files changed, 161 insertions(+)
create mode 100644 test/sql/no-pk-space.result
create mode 100644 test/sql/no-pk-space.test.lua
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 87d4ed4f41..f4d0334f44 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -53,6 +53,12 @@ sql_lookup_space(struct Parse *parse, struct SrcList_item *space_name)
parse->is_aborted = true;
return NULL;
}
+ if (space->index_count == 0 && !space->def->opts.is_view) {
+ diag_set(ClientError, ER_UNSUPPORTED, "SQL",
+ "spaces without primary key");
+ parse->is_aborted = true;
+ return NULL;
+ }
space_name->space = space;
if (sqlIndexedByLookup(parse, space_name) != 0)
space = NULL;
diff --git a/test/sql/errinj.result b/test/sql/errinj.result
index 49e71ff065..a1e7cc4a38 100644
--- a/test/sql/errinj.result
+++ b/test/sql/errinj.result
@@ -340,3 +340,51 @@ errinj.set("ERRINJ_WAL_IO", false)
box.sql.execute("DROP TABLE t3;")
---
...
+-- gh-3780: space without PK raises error if
+-- it is used in SQL queries.
+--
+errinj = box.error.injection
+---
+...
+fiber = require('fiber')
+---
+...
+box.sql.execute("CREATE TABLE t (id INT PRIMARY KEY);")
+---
+...
+box.sql.execute("INSERT INTO t VALUES (1);")
+---
+...
+errinj.set("ERRINJ_WAL_DELAY", true)
+---
+- ok
+...
+-- DROP TABLE consists of several steps: firstly indexes
+-- are deleted, then space itself. Lets make sure that if
+-- first part of drop is successfully finished, but resulted
+-- in yield, all operations on space will be blocked due to
+-- absence of primary key.
+--
+function drop_table_yield() box.sql.execute("DROP TABLE t;") end
+---
+...
+f = fiber.create(drop_table_yield)
+---
+...
+box.sql.execute("SELECT * FROM t;")
+---
+- error: SQL does not support spaces without primary key
+...
+box.sql.execute("INSERT INTO t VALUES (2);")
+---
+- error: SQL does not support spaces without primary key
+...
+box.sql.execute("UPDATE t SET id = 2;")
+---
+- error: SQL does not support spaces without primary key
+...
+-- Finish drop space.
+errinj.set("ERRINJ_WAL_DELAY", false)
+---
+- ok
+...
diff --git a/test/sql/errinj.test.lua b/test/sql/errinj.test.lua
index 80c8fde266..d8833feb44 100644
--- a/test/sql/errinj.test.lua
+++ b/test/sql/errinj.test.lua
@@ -117,3 +117,25 @@ box.sql.execute("ALTER TABLE t3 DROP CONSTRAINT fk1;")
box.sql.execute("INSERT INTO t3 VALUES(1, 1, 3);")
errinj.set("ERRINJ_WAL_IO", false)
box.sql.execute("DROP TABLE t3;")
+
+-- gh-3780: space without PK raises error if
+-- it is used in SQL queries.
+--
+errinj = box.error.injection
+fiber = require('fiber')
+box.sql.execute("CREATE TABLE t (id INT PRIMARY KEY);")
+box.sql.execute("INSERT INTO t VALUES (1);")
+errinj.set("ERRINJ_WAL_DELAY", true)
+-- DROP TABLE consists of several steps: firstly indexes
+-- are deleted, then space itself. Lets make sure that if
+-- first part of drop is successfully finished, but resulted
+-- in yield, all operations on space will be blocked due to
+-- absence of primary key.
+--
+function drop_table_yield() box.sql.execute("DROP TABLE t;") end
+f = fiber.create(drop_table_yield)
+box.sql.execute("SELECT * FROM t;")
+box.sql.execute("INSERT INTO t VALUES (2);")
+box.sql.execute("UPDATE t SET id = 2;")
+-- Finish drop space.
+errinj.set("ERRINJ_WAL_DELAY", false)
diff --git a/test/sql/no-pk-space.result b/test/sql/no-pk-space.result
new file mode 100644
index 0000000000..1d57d16879
--- /dev/null
+++ b/test/sql/no-pk-space.result
@@ -0,0 +1,61 @@
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+---
+...
+format = {}
+---
+...
+format[1] = {'id', 'integer'}
+---
+...
+s = box.schema.create_space('test', {format = format})
+---
+...
+box.sql.execute("SELECT * FROM \"test\";")
+---
+- error: SQL does not support spaces without primary key
+...
+box.sql.execute("INSERT INTO \"test\" VALUES (1);")
+---
+- error: SQL does not support spaces without primary key
+...
+box.sql.execute("DELETE FROM \"test\";")
+---
+- error: SQL does not support spaces without primary key
+...
+box.sql.execute("UPDATE \"test\" SET id = 3;")
+---
+- error: SQL does not support spaces without primary key
+...
+s:drop()
+---
+...
+-- Notorious artefact: check of view referencing counter occurs
+-- after drop of indexes. So, if space:drop() fails due to being
+-- referenced by a view, space becomes unusable in SQL terms.
+--
+box.sql.execute("CREATE TABLE t1 (id INT PRIMARY KEY);")
+---
+...
+box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;")
+---
+...
+box.space.T1:drop()
+---
+- error: 'Can''t drop space ''T1'': other views depend on this space'
+...
+box.sql.execute("SELECT * FROM v1;")
+---
+- error: SQL does not support spaces without primary key
+...
+box.space.V1:drop()
+---
+...
+box.space.T1:drop()
+---
+...
diff --git a/test/sql/no-pk-space.test.lua b/test/sql/no-pk-space.test.lua
new file mode 100644
index 0000000000..2828777f7d
--- /dev/null
+++ b/test/sql/no-pk-space.test.lua
@@ -0,0 +1,24 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+format = {}
+format[1] = {'id', 'integer'}
+s = box.schema.create_space('test', {format = format})
+box.sql.execute("SELECT * FROM \"test\";")
+box.sql.execute("INSERT INTO \"test\" VALUES (1);")
+box.sql.execute("DELETE FROM \"test\";")
+box.sql.execute("UPDATE \"test\" SET id = 3;")
+
+s:drop()
+
+-- Notorious artefact: check of view referencing counter occurs
+-- after drop of indexes. So, if space:drop() fails due to being
+-- referenced by a view, space becomes unusable in SQL terms.
+--
+box.sql.execute("CREATE TABLE t1 (id INT PRIMARY KEY);")
+box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;")
+box.space.T1:drop()
+box.sql.execute("SELECT * FROM v1;")
+box.space.V1:drop()
+box.space.T1:drop()
--
GitLab