From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 3221C29208 for ; Tue, 19 Mar 2019 16:25:57 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Y61gG4N3GyWM for ; Tue, 19 Mar 2019 16:25:57 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 8124323329 for ; Tue, 19 Mar 2019 16:25:56 -0400 (EDT) From: Nikita Pettik Subject: [tarantool-patches] [PATCH] sql: make spaces without PK illegal in queries Date: Tue, 19 Mar 2019 23:25:52 +0300 Message-Id: <20190319202552.93360-1-korablev@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Nikita Pettik 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