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 88C1C24DE9 for ; Sat, 27 Jul 2019 06:29:29 -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 PQzdmD8Nxqr8 for ; Sat, 27 Jul 2019 06:29:29 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 2503620DCF for ; Sat, 27 Jul 2019 06:29:29 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v1 1/1] sql: change the way to set AUTOINCREMENT Date: Sat, 27 Jul 2019 13:29:26 +0300 Message-Id: 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: korablev@tarantool.org Cc: tarantool-patches@freelists.org Prior to this patch, the auto-increment feature could only be set in an INTEGER field of PRIMARY KEY if the PRIMARY KEY consisted of a single field. It was not possible to use this feature if the PRIMARY KEY consists of more than one field. This patch allows to set AUTOINCREMENT for any INTEGER field of PRIMARY KEY. This was achieved by changing the way the feature is defined. It was previously defined after the PRIMARY KEY keywords, but now it must follow the definition of the field to which it belongs. Example of old definition: CREATE TABLE t (i INT, a INT, PRIMARY KEY (a AUTOINCREMENT)); Example of new definition: CREATE TABLE t (i INT, a INT AUTOINCREMENT, PRIMARY KEY (a)); Closes #4217 @TarantoolBot document Title: The auto-increment feature The auto-increment feature allows to replace a NULL inserted in an INTEGER field of PRIMARY KEY with a number generated by sequence if the field has the feature. This feature can belong to any INTEGER field of PRIMARY KEY, but no more than one of the INTEGER fields of PRIMARY KEY can have it. Definition of the feature should follow the field it belongs to. Examples of definition of auto-increment feature: CREATE TABLE t (i INT PRIMARY KEY AUTOINCREMENT); CREATE TABLE t (i INT AUTOINCREMENT, a INT, PRIMARY KEY(i)); CREATE TABLE t (i INT, a INT AUTOINCREMENT, PRIMARY KEY (i, a)); --- https://github.com/tarantool/tarantool/issues/4217 https://github.com/tarantool/tarantool/tree/imeevma/gh-4217-autoinc-on-any-field-of-pk src/box/sql/build.c | 33 +++------ src/box/sql/parse.y | 21 +++--- src/box/sql/parse_def.h | 9 ++- test/sql-tap/autoinc.test.lua | 148 +++++++++++++++++++++++++++++++++++++-- test/sql-tap/sql-errors.test.lua | 2 +- 5 files changed, 172 insertions(+), 41 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 0a6759e..53f21da 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -625,12 +625,6 @@ sqlAddPrimaryKey(struct Parse *pParse) sql_create_index(pParse); if (db->mallocFailed) goto primary_key_exit; - } else if (pParse->create_table_def.has_autoinc) { - diag_set(ClientError, ER_CREATE_SPACE, space->def->name, - "AUTOINCREMENT is only allowed on an INTEGER PRIMARY "\ - "KEY or INT PRIMARY KEY"); - pParse->is_aborted = true; - goto primary_key_exit; } else { sql_create_index(pParse); pList = NULL; @@ -1049,18 +1043,9 @@ emitNewSysSequenceRecord(Parse *pParse, int reg_seq_id, const char *seq_name) } static int -emitNewSysSpaceSequenceRecord(Parse *pParse, int reg_space_id, int reg_seq_id, - struct index_def *idx_def) +emitNewSysSpaceSequenceRecord(Parse *pParse, int reg_space_id, int reg_seq_id) { - struct key_part *part = &idx_def->key_def->parts[0]; - int fieldno = part->fieldno; - char *path = NULL; - if (part->path != NULL) { - path = sqlDbStrNDup(pParse->db, part->path, part->path_len); - if (path == NULL) - return -1; - path[part->path_len] = 0; - } + uint32_t autoinc_field = pParse->create_table_def.autoinc_field; Vdbe *v = sqlGetVdbe(pParse); int first_col = pParse->nMem + 1; @@ -1076,12 +1061,10 @@ emitNewSysSpaceSequenceRecord(Parse *pParse, int reg_space_id, int reg_seq_id, sqlVdbeAddOp2(v, OP_Bool, true, first_col + 3); /* 4. Field id. */ - sqlVdbeAddOp2(v, OP_Integer, fieldno, first_col + 4); + sqlVdbeAddOp2(v, OP_Integer, autoinc_field, first_col + 4); /* 5. Field path. */ - sqlVdbeAddOp4(v, OP_String8, 0, first_col + 5, 0, - path != NULL ? path : "", - path != NULL ? P4_DYNAMIC : P4_STATIC ); + sqlVdbeAddOp4(v, OP_String8, 0, first_col + 5, 0, "", P4_STATIC); sqlVdbeAddOp3(v, OP_MakeRecord, first_col + 1, 5, first_col); return first_col; @@ -1350,7 +1333,7 @@ sqlEndTable(struct Parse *pParse) * Check to see if we need to create an _sequence table * for keeping track of autoincrement keys. */ - if (pParse->create_table_def.has_autoinc) { + if (pParse->create_table_def.autoinc_field != UINT32_MAX) { assert(reg_space_id != 0); /* Do an insertion into _sequence. */ int reg_seq_id = ++pParse->nMem; @@ -1363,9 +1346,9 @@ sqlEndTable(struct Parse *pParse) save_record(pParse, BOX_SEQUENCE_ID, reg_seq_record + 1, 1, v->nOp - 1, true); /* Do an insertion into _space_sequence. */ - int reg_space_seq_record = emitNewSysSpaceSequenceRecord(pParse, - reg_space_id, reg_seq_id, - new_space->index[0]->def); + int reg_space_seq_record = + emitNewSysSpaceSequenceRecord(pParse, reg_space_id, + reg_seq_id); sqlVdbeAddOp3(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID, 0, reg_space_seq_record); save_record(pParse, BOX_SPACE_SEQUENCE_ID, diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index d4e1ec8..be0f27a 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -266,9 +266,18 @@ ccons ::= NULL onconf(R). { if (R != ON_CONFLICT_ACTION_ABORT) sql_column_add_nullable_action(pParse, R); } +ccons ::= AUTOINCR. { + if (pParse->create_table_def.autoinc_field != UINT32_MAX) { + diag_set(ClientError, ER_SQL_SYNTAX, "CREATE TABLE", "statement cannot " + "have more than one AUTOINCREMENT"); + pParse->is_aborted = true; + } else { + pParse->create_table_def.autoinc_field = + pParse->create_table_def.new_space->def->field_count - 1; + } +} ccons ::= NOT NULL onconf(R). {sql_column_add_nullable_action(pParse, R);} -ccons ::= cconsname(N) PRIMARY KEY sortorder(Z) autoinc(I). { - pParse->create_table_def.has_autoinc = I; +ccons ::= cconsname(N) PRIMARY KEY sortorder(Z). { create_index_def_init(&pParse->create_index_def, NULL, &N, NULL, SQL_INDEX_TYPE_CONSTRAINT_PK, Z, false); sqlAddPrimaryKey(pParse); @@ -295,11 +304,6 @@ ccons ::= cconsname(N) REFERENCES nm(T) eidlist_opt(TA) matcharg(M) refargs(R). ccons ::= defer_subclause(D). {fk_constraint_change_defer_mode(pParse, D);} ccons ::= COLLATE id(C). {sqlAddCollateType(pParse, &C);} -// The optional AUTOINCREMENT keyword -%type autoinc {int} -autoinc(X) ::= . {X = 0;} -autoinc(X) ::= AUTOINCR. {X = 1;} - // The next group of rules parses the arguments to a REFERENCES clause // that determine if the referential integrity checking is deferred or // or immediate and which determine what action to take if a ref-integ @@ -337,8 +341,7 @@ init_deferred_pred_opt(A) ::= . {A = 0;} init_deferred_pred_opt(A) ::= INITIALLY DEFERRED. {A = 1;} init_deferred_pred_opt(A) ::= INITIALLY IMMEDIATE. {A = 0;} -tcons ::= cconsname(N) PRIMARY KEY LP sortlist(X) autoinc(I) RP. { - pParse->create_table_def.has_autoinc = I; +tcons ::= cconsname(N) PRIMARY KEY LP sortlist(X) RP. { create_index_def_init(&pParse->create_index_def, NULL, &N, X, SQL_INDEX_TYPE_CONSTRAINT_PK, SORT_ORDER_ASC, false); sqlAddPrimaryKey(pParse); diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h index 557e415..7d4bca3 100644 --- a/src/box/sql/parse_def.h +++ b/src/box/sql/parse_def.h @@ -212,8 +212,12 @@ struct create_table_def { uint32_t check_count; /** Check constraint appeared in CREATE TABLE stmt. */ struct rlist new_check; - /** True, if table to be created has AUTOINCREMENT PK. */ - bool has_autoinc; + /** + * Field number of field with AUTOINCREMENT. If its value + * is UINT32_MAX than there is no field with + * AUTOINCREMENT. + */ + uint32_t autoinc_field; }; struct create_view_def { @@ -461,6 +465,7 @@ create_table_def_init(struct create_table_def *table_def, struct Token *name, if_not_exists); rlist_create(&table_def->new_fkey); rlist_create(&table_def->new_check); + table_def->autoinc_field = UINT32_MAX; } static inline void diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua index 2601968..05d3042 100755 --- a/test/sql-tap/autoinc.test.lua +++ b/test/sql-tap/autoinc.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(47) +test:plan(58) --!./tcltestrunner.lua -- 2004 November 12 @@ -537,11 +537,13 @@ test:do_catchsql_test( -- Allow the AUTOINCREMENT keyword inside the parentheses -- on a separate PRIMARY KEY designation. +-- UPDATE: Changed in #4217. Now AUTOINCREMENT must follow the +-- definition of the column to which it belongs. -- test:do_execsql_test( "autoinc-7.1", [[ - CREATE TABLE t7(x INTEGER, y REAL, PRIMARY KEY(x AUTOINCREMENT)); + CREATE TABLE t7(x INTEGER AUTOINCREMENT, y REAL, PRIMARY KEY(x)); INSERT INTO t7(y) VALUES(123); INSERT INTO t7(y) VALUES(234); DELETE FROM t7; @@ -561,7 +563,7 @@ test:do_catchsql_test( CREATE TABLE t8(x TEXT PRIMARY KEY AUTOINCREMENT); ]], { -- - 1, "Failed to create space 'T8': AUTOINCREMENT is only allowed on an INTEGER PRIMARY KEY or INT PRIMARY KEY" + 1, "Can't create or modify index 'pk_unnamed_T8_1' in space 'T8': sequence cannot be used with a non-integer key" -- }) @@ -627,7 +629,7 @@ test:do_test( function() return test:execsql([[ DROP TABLE IF EXISTS t7; - CREATE TABLE t7(x INT, y REAL, PRIMARY KEY(x AUTOINCREMENT)); + CREATE TABLE t7(x INT AUTOINCREMENT, y REAL, PRIMARY KEY(x)); INSERT INTO t7(y) VALUES(123); INSERT INTO t7(y) VALUES(234); DELETE FROM t7; @@ -814,4 +816,142 @@ test:do_catchsql_test( -- }) +-- +-- gh-4217: make sure that AUTOINCREMENT can be used for any +-- INTEGER field of PRIMARY KEY. +-- + +-- +-- Make sure AUTOINCREMENT works for a PRIMARY KEY that contains +-- one column. +-- +test:do_execsql_test( + "autoinc-11.1", + [[ + CREATE TABLE t (i INT PRIMARY KEY AUTOINCREMENT, a INT); + INSERT INTO t VALUES (NULL, 1), (NULL, 1), (NULL, 1); + SELECT * FROM t; + ]], { + -- + 1, 1, 2, 1, 3, 1 + -- + }) + +test:do_execsql_test( + "autoinc-11.2", + [[ + DROP TABLE IF EXISTS t; + CREATE TABLE t (i INT AUTOINCREMENT, a INT, PRIMARY KEY(i)); + INSERT INTO t VALUES (NULL, 1), (NULL, 1), (NULL, 1); + SELECT * FROM t; + ]], { + -- + 1, 1, 2, 1, 3, 1 + -- + }) + +-- +-- Make sure AUTOINCREMENT works for any INTEGER field of PRIMARY +-- KEY that contains more than one column. +-- +test:do_execsql_test( + "autoinc-11.3", + [[ + DROP TABLE t; + CREATE TABLE t (i TEXT, a INT AUTOINCREMENT, PRIMARY KEY (i, a)); + INSERT INTO t VALUES (1, NULL), (1, NULL), (1, NULL); + SELECT * FROM t; + ]], { + -- + "1", 1, "1", 2, "1", 3 + -- + }) + +test:do_execsql_test( + "autoinc-11.4", + [[ + DROP TABLE t; + CREATE TABLE t (i TEXT, a INT AUTOINCREMENT, PRIMARY KEY (a, i)); + INSERT INTO t VALUES (1, NULL), (1, NULL), (1, NULL); + SELECT * FROM t; + ]], { + -- + "1", 1, "1", 2, "1", 3 + -- + }) + +-- Make sure that AUTOINCREMENT only works for PRIMARY KEY. +test:do_catchsql_test( + "autoinc-11.5", + [[ + DROP TABLE t; + CREATE TABLE t (i INT PRIMARY KEY, a INT AUTOINCREMENT); + ]], { + -- + 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence field must be a part of the index" + -- + }) + +test:do_catchsql_test( + "autoinc-11.6", + [[ + CREATE TABLE t (i INT, a INT, b INT AUTOINCREMENT, PRIMARY KEY (a, i)); + ]], { + -- + 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence field must be a part of the index" + -- + }) + +-- Make sure that AUTOINCREMENT only works for INTEGER field. +test:do_catchsql_test( + "autoinc-11.7", + [[ + CREATE TABLE t (i TEXT PRIMARY KEY AUTOINCREMENT); + ]], { + -- + 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence cannot be used with a non-integer key" + -- + }) + +test:do_catchsql_test( + "autoinc-11.8", + [[ + CREATE TABLE t (i REAL PRIMARY KEY AUTOINCREMENT); + ]], { + -- + 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence cannot be used with a non-integer key" + -- + }) + +test:do_catchsql_test( + "autoinc-11.9", + [[ + CREATE TABLE t (i BOOLEAN PRIMARY KEY AUTOINCREMENT); + ]], { + -- + 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence cannot be used with a non-integer key" + -- + }) + +test:do_catchsql_test( + "autoinc-11.10", + [[ + CREATE TABLE t (i SCALAR PRIMARY KEY AUTOINCREMENT); + ]], { + -- + 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence cannot be used with a non-integer key" + -- + }) + +-- Make sure that there can be no more than one AUTOINCREMENT. +test:do_catchsql_test( + "autoinc-11.11", + [[ + CREATE TABLE t (i INT AUTOINCREMENT, a INT AUTOINCREMENT, PRIMARY KEY (i, a)); + ]], { + -- + 1, "Syntax error in CREATE TABLE: statement cannot have more than one AUTOINCREMENT" + -- + }) + test:finish_test() diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua index f452e3c..a93b6ab 100755 --- a/test/sql-tap/sql-errors.test.lua +++ b/test/sql-tap/sql-errors.test.lua @@ -62,7 +62,7 @@ test:do_catchsql_test( CREATE TABLE t5 (i TEXT PRIMARY KEY AUTOINCREMENT); ]], { -- - 1,"Failed to create space 'T5': AUTOINCREMENT is only allowed on an INTEGER PRIMARY KEY or INT PRIMARY KEY" + 1,"Can't create or modify index 'pk_unnamed_T5_1' in space 'T5': sequence cannot be used with a non-integer key" -- }) -- 2.7.4