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 72C3C24843 for ; Wed, 9 Jan 2019 07:13:32 -0500 (EST) 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 U8n79jSWaqEG for ; Wed, 9 Jan 2019 07:13:32 -0500 (EST) Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (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 0DBAF24751 for ; Wed, 9 Jan 2019 07:13:32 -0500 (EST) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 6/6] sql: introduce ALTER TABLE ADD CONSTRAINT UNIQUE/PRIMARY KEY Date: Wed, 9 Jan 2019 14:13:20 +0200 Message-Id: <5b411f9988366aee0418ed4aeb01243a85cfbd03.1547035184.git.korablev@tarantool.org> In-Reply-To: References: In-Reply-To: References: 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 Cc: v.shpilevoy@tarantool.org, Nikita Pettik Table (aka space) can be created without indexes at least from Lua-land (note that according ANSI SQL table may lack PK). Since there were no facilities to create primary key constraint on already existing table, lets extend ADD CONSTRAINT statement with UNIQUE and PRIMARY KEY clauses. In this case, UNIQUE works exactly in the same way as CREATE UNIQUE INDEX ... statement does. In Tarantool primary index is an index with id == 0, so during execution of ADD CONSTRAINT PRIMARY KEY we check that there is no any entries in _index space and create that one. Otherwise, error is raised. Part of #3097 Follow-up #3914 --- src/box/sql/build.c | 29 +++++++++++++++++++++-- src/box/sql/parse.y | 15 ++++++++++++ test/sql-tap/alter.test.lua | 58 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 99 insertions(+), 3 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 1d7b6c827..0314be957 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2083,6 +2083,19 @@ generate_index_id(struct Parse *parse, uint32_t space_id, int cursor) return iid_reg; } +static void +pk_check_existence(struct Parse *parse, uint32_t space_id, int cursor) +{ + struct Vdbe *v = sqlite3GetVdbe(parse); + int tmp_reg = ++parse->nMem; + sqlite3VdbeAddOp2(v, OP_Integer, space_id, tmp_reg); + int found_addr = sqlite3VdbeAddOp4Int(v, OP_NotFound, cursor, 0, + tmp_reg, 1); + sqlite3VdbeAddOp4(v, OP_Halt, SQLITE_ERROR, ON_CONFLICT_ACTION_FAIL, 0, + "multiple primary keys are not allowed", P4_STATIC); + sqlite3VdbeJumpHere(v, found_addr); +} + /** * Add new index to table's indexes list. * We follow convention that PK comes first in list. @@ -2520,8 +2533,20 @@ sql_create_index(struct Parse *parse, struct Token *token, (void *)space_by_id(BOX_INDEX_ID), P4_SPACEPTR); sqlite3VdbeChangeP5(vdbe, OPFLAG_SEEKEQ); - - int index_id = generate_index_id(parse, def->id, cursor); + int index_id; + /* + * In case we are creating PRIMARY KEY constraint + * (via ALTER TABLE) we must ensure that table + * doesn't feature any indexes. Otherwise, + * we can immediately halt execution of VDBE. + */ + if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK) { + pk_check_existence(parse, def->id, cursor); + index_id = parse->nMem++; + sqlite3VdbeAddOp2(vdbe, OP_Integer, 0, index_id); + } else { + index_id = generate_index_id(parse, def->id, cursor); + } sqlite3VdbeAddOp1(vdbe, OP_Close, cursor); vdbe_emit_create_index(parse, def, index->def, def->id, index_id); diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 0b1cef597..82aeee2b5 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -1466,6 +1466,7 @@ add_constraint_start ::= ADD CONSTRAINT nm(Z). { } constraint_def ::= foreign_key_def. +constraint_def ::= unique_def. foreign_key_def ::= FOREIGN KEY LP eidlist(FA) RP REFERENCES nm(T) eidlist_opt(TA) refargs(R) defer_subclause_opt(D). { @@ -1473,6 +1474,20 @@ foreign_key_def ::= FOREIGN KEY LP eidlist(FA) RP REFERENCES nm(T) } +unique_def ::= unique_spec(U) LP sortlist(X) RP. { + sql_create_index(pParse, &pParse->constraint->name, + pParse->constraint->table_name, X, SORT_ORDER_ASC, false, U); + } + +%type unique_spec {int} +unique_spec(U) ::= UNIQUE. { U = SQL_INDEX_TYPE_CONSTRAINT_UNIQUE; } +unique_spec(U) ::= PRIMARY KEY. { U = SQL_INDEX_TYPE_CONSTRAINT_PK; } + +/** + * Currently, to drop unique constraint it is required + * to use drop index (since fk and unique constraints don't + * share name space). + */ drop_constraint_def ::= DROP CONSTRAINT nm(Z). { sql_drop_foreign_key(pParse, &Z); } diff --git a/test/sql-tap/alter.test.lua b/test/sql-tap/alter.test.lua index 1aad555c0..925753749 100755 --- a/test/sql-tap/alter.test.lua +++ b/test/sql-tap/alter.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(43) +test:plan(50) test:do_execsql_test( "alter-1.1", @@ -517,6 +517,62 @@ test:do_catchsql_test( -- }) +test:do_test( + "alter-8.1.0", + function() + format = {} + format[1] = { name = 'id', type = 'scalar'} + format[2] = { name = 'f2', type = 'scalar'} + s = box.schema.create_space('T', {format = format}) + end, + {}) + +test:do_catchsql_test( + "alter-8.1.1", + [[ + ALTER TABLE t ADD CONSTRAINT pk PRIMARY KEY("id"); + ]], { + 0 + }) + +test:do_test( + "alter-8.1.2", + function() + i = box.space.T.index[0] + return i.id + end, 0) + +test:do_catchsql_test( + "alter-8.2", + [[ + ALTER TABLE t ADD CONSTRAINT pk1 PRIMARY KEY("f2"); + ]], { + 1, "multiple primary keys are not allowed" + }) + +test:do_catchsql_test( + "alter-8.3.1", + [[ + ALTER TABLE t ADD CONSTRAINT i1 UNIQUE("f2"); + ]], { + 0 + }) + +test:do_test( + "alter-8.3.2", + function() + i = box.space.T.index[1] + return i.id + end, 1) + +test:do_catchsql_test( + "alter-8.4", + [[ + DROP INDEX i1 ON t; + DROP INDEX pk ON t; + ]], { + 0 +}) -- Commented due to #2953 -- -- 2.15.1