[Tarantool-patches] [PATCH 2/2] sql: drop only generated sequence in DROP TABLE

[Chris Sosnin <k.sosnin@tarantool.org>]

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.

Follow up #4546
---
branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4546-sql-drop-grants
issue: https://github.com/tarantool/tarantool/issues/4546

 src/box/sql/build.c                       | 35 ++++++----
 test/sql/autoincrement.result             | 80 +++++++++++++++++++++++
 test/sql/autoincrement.test.lua           | 33 ++++++++++
 test/sql/gh-4546-sql-drop-grants.result   | 16 +++++
 test/sql/gh-4546-sql-drop-grants.test.lua |  5 ++
 5 files changed, 155 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 f1433645a..93aae0894 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 000000000..ca3d6f3cf
--- /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 000000000..63a902aea
--- /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')
diff --git a/test/sql/gh-4546-sql-drop-grants.result b/test/sql/gh-4546-sql-drop-grants.result
index 1079d2ab4..9376c367d 100644
--- a/test/sql/gh-4546-sql-drop-grants.result
+++ b/test/sql/gh-4546-sql-drop-grants.result
@@ -20,6 +20,12 @@ test_space = box.schema.create_space('T', {		\
 })
 ---
 ...
+seq = box.schema.sequence.create('S')
+---
+...
+ind = test_space:create_index('I', {sequence = 'S'})
+---
+...
 box.schema.user.grant('test_user1', 'read', 'space', 'T')
 ---
 ...
@@ -36,3 +42,13 @@ box.schema.user.drop('test_user1')
 box.schema.user.drop('test_user2')
 ---
 ...
+seqs = box.space._sequence:select{}
+---
+...
+#seqs == 1 and seqs[1].name == seq.name or seqs
+---
+- true
+...
+seq:drop()
+---
+...
diff --git a/test/sql/gh-4546-sql-drop-grants.test.lua b/test/sql/gh-4546-sql-drop-grants.test.lua
index 09cfeda19..ddb8cb0bb 100644
--- a/test/sql/gh-4546-sql-drop-grants.test.lua
+++ b/test/sql/gh-4546-sql-drop-grants.test.lua
@@ -12,8 +12,13 @@ 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('test_user1', 'read', 'space', 'T')
 box.schema.user.grant('test_user2', 'write', 'space', 'T')
 box.execute([[DROP TABLE T;]])
 box.schema.user.drop('test_user1')
 box.schema.user.drop('test_user2')
+seqs = box.space._sequence:select{}
+#seqs == 1 and seqs[1].name == seq.name or seqs
+seq:drop()
-- 
2.21.0 (Apple Git-122.2)