From a1155c8ba5f52211be7d14dfe38e70684dbfd7d7 Mon Sep 17 00:00:00 2001
From: Chris Sosnin <k.sosnin@tarantool.org>
Date: Tue, 24 Dec 2019 23:29:48 +0300
Subject: [PATCH] sql: drop only generated sequence in DROP TABLE

It is possible to create a sequence manually, and give it to a newly
created index as a source of unique identifiers. Such sequences are not
owned by a space, and therefore shouldn't be deleted when the space is
dropped. They are not dropped when space:drop() in Lua is called, but
were dropped in SQL 'DROP TABLE' before this patch. Now Lua and SQL are
consistent in that case.
---
 src/box/sql/build.c             | 35 +++++++++------
 test/sql/autoincrement.result   | 80 +++++++++++++++++++++++++++++++++
 test/sql/autoincrement.test.lua | 33 ++++++++++++++
 3 files changed, 134 insertions(+), 14 deletions(-)
 create mode 100644 test/sql/autoincrement.result
 create mode 100644 test/sql/autoincrement.test.lua

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 8d282edfd6..a106dc3d50 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1611,26 +1611,33 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
 	sqlVdbeAddOp2(v, OP_Integer, space_id, space_id_reg);
 	sqlVdbeAddOp1(v, OP_CheckViewReferences, space_id_reg);
 	if (space->sequence != NULL) {
-		/* Delete entry from _sequence_data. */
-		int sequence_id_reg = ++parse_context->nMem;
-		sqlVdbeAddOp2(v, OP_Integer, space->sequence->def->id,
-				  sequence_id_reg);
-		sqlVdbeAddOp3(v, OP_MakeRecord, sequence_id_reg, 1,
-				  idx_rec_reg);
-		sqlVdbeAddOp2(v, OP_SDelete, BOX_SEQUENCE_DATA_ID,
-				  idx_rec_reg);
-		VdbeComment((v, "Delete entry from _sequence_data"));
 		/* Delete entry from _space_sequence. */
 		sqlVdbeAddOp3(v, OP_MakeRecord, space_id_reg, 1,
 				  idx_rec_reg);
 		sqlVdbeAddOp2(v, OP_SDelete, BOX_SPACE_SEQUENCE_ID,
 				  idx_rec_reg);
 		VdbeComment((v, "Delete entry from _space_sequence"));
-		/* Delete entry by id from _sequence. */
-		sqlVdbeAddOp3(v, OP_MakeRecord, sequence_id_reg, 1,
-				  idx_rec_reg);
-		sqlVdbeAddOp2(v, OP_SDelete, BOX_SEQUENCE_ID, idx_rec_reg);
-		VdbeComment((v, "Delete entry from _sequence"));
+		if (space->sequence->is_generated) {
+			/* Delete entry from _sequence_data. */
+			int sequence_id_reg = ++parse_context->nMem;
+			sqlVdbeAddOp2(v, OP_Integer, space->sequence->def->id,
+				      sequence_id_reg);
+			sqlVdbeAddOp3(v, OP_MakeRecord, sequence_id_reg, 1,
+				      idx_rec_reg);
+			sqlVdbeAddOp2(v, OP_SDelete, BOX_SEQUENCE_DATA_ID,
+				      idx_rec_reg);
+			VdbeComment((v, "Delete entry from _sequence_data"));
+			/* Delete entries from _priv */
+		        vdbe_emit_revoke_object(parse_context, "sequence",
+						space->sequence->def->id,
+						space->sequence->access);
+			/* Delete entry by id from _sequence. */
+			sqlVdbeAddOp3(v, OP_MakeRecord, sequence_id_reg, 1,
+				      idx_rec_reg);
+			sqlVdbeAddOp2(v, OP_SDelete, BOX_SEQUENCE_ID,
+				      idx_rec_reg);
+			VdbeComment((v, "Delete entry from _sequence"));
+		}
 	}
 	/* Delete all child FK constraints. */
 	struct fk_constraint *child_fk;
diff --git a/test/sql/autoincrement.result b/test/sql/autoincrement.result
new file mode 100644
index 0000000000..ca3d6f3cf9
--- /dev/null
+++ b/test/sql/autoincrement.result
@@ -0,0 +1,80 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+engine = test_run:get_cfg('engine')
+ | ---
+ | ...
+
+--
+-- Not automatic sequence should not be removed by DROP TABLE.
+--
+box.schema.user.create('user1')
+ | ---
+ | ...
+test_space = box.schema.create_space('T', {         \
+    engine = engine,                                \
+    format = {{'i', 'integer'}}                     \
+})
+ | ---
+ | ...
+seq = box.schema.sequence.create('S')
+ | ---
+ | ...
+ind = test_space:create_index('I', {sequence = 'S'})
+ | ---
+ | ...
+box.schema.user.grant('user1', 'write', 'sequence', 'S')
+ | ---
+ | ...
+box.execute('DROP TABLE t');
+ | ---
+ | - row_count: 1
+ | ...
+seqs = box.space._sequence:select{}
+ | ---
+ | ...
+#seqs == 1 and seqs[1].name == seq.name or seqs
+ | ---
+ | - true
+ | ...
+seq:drop()
+ | ---
+ | ...
+
+--
+-- Automatic sequence should be removed at DROP TABLE, together
+-- with all the grants.
+--
+box.execute('CREATE TABLE t (id INTEGER PRIMARY KEY AUTOINCREMENT)')
+ | ---
+ | - row_count: 1
+ | ...
+seqs = box.space._sequence:select{}
+ | ---
+ | ...
+#seqs == 1 or seqs
+ | ---
+ | - true
+ | ...
+seq = seqs[1].name
+ | ---
+ | ...
+box.schema.user.grant('user1', 'write', 'sequence', seq)
+ | ---
+ | ...
+box.execute('DROP TABLE t')
+ | ---
+ | - row_count: 1
+ | ...
+seqs = box.space._sequence:select{}
+ | ---
+ | ...
+#seqs == 0 or seqs
+ | ---
+ | - true
+ | ...
+
+box.schema.user.drop('user1')
+ | ---
+ | ...
diff --git a/test/sql/autoincrement.test.lua b/test/sql/autoincrement.test.lua
new file mode 100644
index 0000000000..63a902aea9
--- /dev/null
+++ b/test/sql/autoincrement.test.lua
@@ -0,0 +1,33 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+
+--
+-- Not automatic sequence should not be removed by DROP TABLE.
+--
+box.schema.user.create('user1')
+test_space = box.schema.create_space('T', {         \
+    engine = engine,                                \
+    format = {{'i', 'integer'}}                     \
+})
+seq = box.schema.sequence.create('S')
+ind = test_space:create_index('I', {sequence = 'S'})
+box.schema.user.grant('user1', 'write', 'sequence', 'S')
+box.execute('DROP TABLE t');
+seqs = box.space._sequence:select{}
+#seqs == 1 and seqs[1].name == seq.name or seqs
+seq:drop()
+
+--
+-- Automatic sequence should be removed at DROP TABLE, together
+-- with all the grants.
+--
+box.execute('CREATE TABLE t (id INTEGER PRIMARY KEY AUTOINCREMENT)')
+seqs = box.space._sequence:select{}
+#seqs == 1 or seqs
+seq = seqs[1].name
+box.schema.user.grant('user1', 'write', 'sequence', seq)
+box.execute('DROP TABLE t')
+seqs = box.space._sequence:select{}
+#seqs == 0 or seqs
+
+box.schema.user.drop('user1')
-- 
GitLab