[tarantool-patches] [PATCH] sql: make spaces without PK illegal in queries

[Nikita Pettik <korablev@tarantool.org>]

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
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-3780-dissalow-spaces-without-pk
Issue: https://github.com/tarantool/tarantool/issues/3780

src/box/sql/delete.c          |  7 +++++
 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, 162 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 5170c7f59..2b5c5014e 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -54,6 +54,13 @@ sql_lookup_space(struct Parse *parse, struct SrcList_item *space_name)
 		parse->nErr++;
 		return NULL;
 	}
+	if (space->index_count == 0 && !space->def->opts.is_view) {
+		diag_set(ClientError, ER_UNSUPPORTED, "SQL",
+			 "spaces without primary key");
+		parse->rc = SQL_TARANTOOL_ERROR;
+		parse->nErr++;
+		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 49e71ff06..a1e7cc4a3 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 80c8fde26..d8833feb4 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 000000000..1d57d1687
--- /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 000000000..2828777f7
--- /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()
-- 
2.15.1