[tarantool-patches] [PATCH v1 1/1] sql: change the way to set AUTOINCREMENT
imeevma at tarantool.org
imeevma at tarantool.org
Sat Jul 27 13:29:26 MSK 2019
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);
]], {
-- <autoinc-7.2>
- 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"
-- </autoinc-7.2>
})
@@ -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(
-- </autoinc-gh-3670>
})
+--
+-- 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;
+ ]], {
+ -- <autoinc-11.1>
+ 1, 1, 2, 1, 3, 1
+ -- </autoinc-11.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;
+ ]], {
+ -- <autoinc-11.1>
+ 1, 1, 2, 1, 3, 1
+ -- </autoinc-11.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;
+ ]], {
+ -- <autoinc-11.3>
+ "1", 1, "1", 2, "1", 3
+ -- </autoinc-11.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;
+ ]], {
+ -- <autoinc-11.4>
+ "1", 1, "1", 2, "1", 3
+ -- </autoinc-11.4>
+ })
+
+-- 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);
+ ]], {
+ -- <autoinc-11.5>
+ 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence field must be a part of the index"
+ -- </autoinc-11.5>
+ })
+
+test:do_catchsql_test(
+ "autoinc-11.6",
+ [[
+ CREATE TABLE t (i INT, a INT, b INT AUTOINCREMENT, PRIMARY KEY (a, i));
+ ]], {
+ -- <autoinc-11.6>
+ 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence field must be a part of the index"
+ -- </autoinc-11.6>
+ })
+
+-- Make sure that AUTOINCREMENT only works for INTEGER field.
+test:do_catchsql_test(
+ "autoinc-11.7",
+ [[
+ CREATE TABLE t (i TEXT PRIMARY KEY AUTOINCREMENT);
+ ]], {
+ -- <autoinc-11.7>
+ 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence cannot be used with a non-integer key"
+ -- </autoinc-11.7>
+ })
+
+test:do_catchsql_test(
+ "autoinc-11.8",
+ [[
+ CREATE TABLE t (i REAL PRIMARY KEY AUTOINCREMENT);
+ ]], {
+ -- <autoinc-11.7>
+ 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence cannot be used with a non-integer key"
+ -- </autoinc-11.7>
+ })
+
+test:do_catchsql_test(
+ "autoinc-11.9",
+ [[
+ CREATE TABLE t (i BOOLEAN PRIMARY KEY AUTOINCREMENT);
+ ]], {
+ -- <autoinc-11.7>
+ 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence cannot be used with a non-integer key"
+ -- </autoinc-11.7>
+ })
+
+test:do_catchsql_test(
+ "autoinc-11.10",
+ [[
+ CREATE TABLE t (i SCALAR PRIMARY KEY AUTOINCREMENT);
+ ]], {
+ -- <autoinc-11.7>
+ 1, "Can't create or modify index 'pk_unnamed_T_1' in space 'T': sequence cannot be used with a non-integer key"
+ -- </autoinc-11.7>
+ })
+
+-- 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));
+ ]], {
+ -- <autoinc-11.7>
+ 1, "Syntax error in CREATE TABLE: statement cannot have more than one AUTOINCREMENT"
+ -- </autoinc-11.7>
+ })
+
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);
]], {
-- <sql-errors-1.5>
- 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"
-- </sql-errors-1.5>
})
--
2.7.4
More information about the Tarantool-patches
mailing list