From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id BF58A4696C4 for ; Wed, 18 Dec 2019 14:01:00 +0300 (MSK) From: Chris Sosnin Date: Wed, 18 Dec 2019 14:00:59 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 2/2] sql: drop only generated sequence in DROP TABLE List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org, korablev@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)