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 C0AB02A43B for ; Wed, 20 Mar 2019 05:38: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 hiXCMKPiNXgL for ; Wed, 20 Mar 2019 05:38:57 -0400 (EDT) Received: from smtp43.i.mail.ru (smtp43.i.mail.ru [94.100.177.103]) (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 DCDEB2A437 for ; Wed, 20 Mar 2019 05:38:56 -0400 (EDT) Received: by smtp43.i.mail.ru with esmtpa (envelope-from ) id 1h6XgU-0001Hu-K5 for tarantool-patches@freelists.org; Wed, 20 Mar 2019 12:38:54 +0300 From: "n.pettik" Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH] sql: make spaces without PK illegal in queries Date: Wed, 20 Mar 2019 12:38:53 +0300 References: <20190319202552.93360-1-korablev@tarantool.org> In-Reply-To: <20190319202552.93360-1-korablev@tarantool.org> Message-Id: <469FED20-34A2-4D35-8322-3EDD4C7E925C@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 > On 19 Mar 2019, at 23:25, Nikita Pettik = wrote: >=20 > 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. >=20 > As a solution to this problem (at least temporary) now we disallow > query processing involving spaces without PK except for views. >=20 > Closes #3780 > --- > Branch: = https://github.com/tarantool/tarantool/tree/np/gh-3780-dissalow-spaces-wit= hout-pk > Issue: https://github.com/tarantool/tarantool/issues/3780 >=20 > 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 >=20 > 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 =3D=3D 0 && !space->def->opts.is_view) { > + diag_set(ClientError, ER_UNSUPPORTED, "SQL", > + "spaces without primary key"); > + parse->rc =3D SQL_TARANTOOL_ERROR; > + parse->nErr++; > + return NULL; > + } After rebase to very fresh 2.1: diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index 5b8f11967..f4d0334f4 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -56,8 +56,7 @@ sql_lookup_space(struct Parse *parse, struct = SrcList_item *space_name) if (space->index_count =3D=3D 0 && !space->def->opts.is_view) { diag_set(ClientError, ER_UNSUPPORTED, "SQL", "spaces without primary key"); - parse->rc =3D SQL_TARANTOOL_ERROR; - parse->nErr++; + parse->is_aborted =3D true; return NULL; } space_name->space =3D space; > space_name->space =3D space; > if (sqlIndexedByLookup(parse, space_name) !=3D 0) > space =3D 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 =3D box.error.injection > +--- > +... > +fiber =3D 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 =3D 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 =3D 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 =3D box.error.injection > +fiber =3D 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 =3D 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 =3D 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 =3D require('test_run').new() > +--- > +... > +engine =3D test_run:get_cfg('engine') > +--- > +... > +box.sql.execute('pragma sql_default_engine=3D\''..engine..'\'') > +--- > +... > +format =3D {} > +--- > +... > +format[1] =3D {'id', 'integer'} > +--- > +... > +s =3D box.schema.create_space('test', {format =3D 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 =3D 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 =3D require('test_run').new() > +engine =3D test_run:get_cfg('engine') > +box.sql.execute('pragma sql_default_engine=3D\''..engine..'\'') > + > +format =3D {} > +format[1] =3D {'id', 'integer'} > +s =3D box.schema.create_space('test', {format =3D 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 =3D 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() > --=20 > 2.15.1 >=20 >=20