From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <tarantool-patches-bounce@freelists.org>
Received: from localhost (localhost [127.0.0.1])
by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id C0AB02A43B
for <tarantool-patches@freelists.org>; 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 <tarantool-patches@freelists.org>;
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 <tarantool-patches@freelists.org>; Wed, 20 Mar 2019 05:38:56 -0400 (EDT)
Received: by smtp43.i.mail.ru with esmtpa (envelope-from <korablev@tarantool.org>)
id 1h6XgU-0001Hu-K5
for tarantool-patches@freelists.org; Wed, 20 Mar 2019 12:38:54 +0300
From: "n.pettik" <korablev@tarantool.org>
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: <mailto:ecartis@freelists.org?Subject=help>
List-Unsubscribe: <mailto:tarantool-patches-request@freelists.org?Subject=unsubscribe>
List-software: Ecartis version 1.0.0
List-Id: tarantool-patches <tarantool-patches.freelists.org>
List-Subscribe: <mailto:tarantool-patches-request@freelists.org?Subject=subscribe>
List-Owner: <mailto:>
List-post: <mailto:tarantool-patches@freelists.org>
List-Archive: <http://www.freelists.org/archives/tarantool-patches>
To: tarantool-patches@freelists.org
> On 19 Mar 2019, at 23:25, Nikita Pettik <korablev@tarantool.org> =
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