From 08c7d7c13051c04d3227bc76e4e9244a7922711c Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Tue, 27 Aug 2019 14:03:58 +0300
Subject: [PATCH] sql: AUTOINCREMENT for multipart PK
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 consisted of more than one field. This patch defines
two ways to set AUTOINCREMENT for any INTEGER or UNSIGNED field of
PRIMARY KEY.
Closes #4217
@TarantoolBot document
Title: The auto-increment feature for multipart PK
The auto-increment feature can be set to any INTEGER or UNSIGNED
field of PRIMARY KEY using one of two ways:
1) AUTOINCREMENT in column definition:
CREATE TABLE t (i INT, a INT AUTOINCREMENT, PRIMARY KEY (i, a));
CREATE TABLE t (i INT AUTOINCREMENT, a INT, PRIMARY KEY (i, a));
2) AUTOINCREMENT in PRIMARY KEY definition:
CREATE TABLE t (i INT, a INT, PRIMARY KEY (i, a AUTOINCREMENT));
CREATE TABLE t (i INT, a INT, PRIMARY KEY (i AUTOINCREMENT, a));
---
src/box/sql/build.c | 68 ++++++++++++++--------
src/box/sql/parse.y | 50 ++++++++++++++--
src/box/sql/parse_def.h | 3 +
src/box/sql/sqlInt.h | 31 ++++++++++
test/sql-tap/autoinc.test.lua | 97 +++++++++++++++++++++++++++++++-
test/sql-tap/sql-errors.test.lua | 2 +-
test/sql-tap/where4.test.lua | 2 +-
7 files changed, 220 insertions(+), 33 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index b4f114ea87..233f567340 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -521,12 +521,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;
@@ -942,18 +936,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 fieldno = pParse->create_table_def.autoinc_fieldno;
Vdbe *v = sqlGetVdbe(pParse);
int first_col = pParse->nMem + 1;
@@ -972,9 +957,7 @@ emitNewSysSpaceSequenceRecord(Parse *pParse, int reg_space_id, int reg_seq_id,
sqlVdbeAddOp2(v, OP_Integer, fieldno, 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;
@@ -1249,9 +1232,9 @@ sqlEndTable(struct Parse *pParse)
new_space->def->name);
sqlVdbeAddOp2(v, OP_SInsert, BOX_SEQUENCE_ID, reg_seq_record);
/* 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);
sqlVdbeAddOp2(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID,
reg_space_seq_record);
}
@@ -3219,3 +3202,42 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id,
sqlVdbeAddOp1(v, OP_Close, cursor);
return 0;
}
+
+int
+sql_add_autoincrement(struct Parse *parse_context, uint32_t fieldno)
+{
+ if (parse_context->create_table_def.has_autoinc) {
+ diag_set(ClientError, ER_SQL_SYNTAX, "CREATE TABLE", "Table "
+ "must feature at most one AUTOINCREMENT field");
+ parse_context->is_aborted = true;
+ return -1;
+ }
+ parse_context->create_table_def.has_autoinc = true;
+ parse_context->create_table_def.autoinc_fieldno = fieldno;
+ return 0;
+}
+
+int
+sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
+ uint32_t *fieldno)
+{
+ struct space_def *def = parse_context->create_table_def.new_space->def;
+ struct Expr *name = sqlExprSkipCollate(field_name);
+ if (name->op != TK_ID) {
+ diag_set(ClientError, ER_INDEX_DEF_UNSUPPORTED, "Expressions");
+ parse_context->is_aborted = true;
+ return -1;
+ }
+ uint32_t i;
+ for (i = 0; i < def->field_count; ++i) {
+ if (strcmp(def->fields[i].name, name->u.zToken) == 0)
+ break;
+ }
+ if (i == def->field_count) {
+ diag_set(ClientError, ER_SQL_CANT_RESOLVE_FIELD, name->u.zToken);
+ parse_context->is_aborted = true;
+ return -1;
+ }
+ *fieldno = i;
+ return 0;
+}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 643e025bdd..ed59a875a4 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -225,8 +225,17 @@ create_table_end ::= . { sqlEndTable(pParse); }
*/
columnlist ::= columnlist COMMA tcons.
-columnlist ::= columnlist COMMA columnname carglist.
-columnlist ::= columnname carglist.
+columnlist ::= columnlist COMMA columnname carglist autoinc(I). {
+ uint32_t fieldno = pParse->create_table_def.new_space->def->field_count - 1;
+ if (I == 1 && sql_add_autoincrement(pParse, fieldno) != 0)
+ return;
+}
+
+columnlist ::= columnname carglist autoinc(I). {
+ uint32_t fieldno = pParse->create_table_def.new_space->def->field_count - 1;
+ if (I == 1 && sql_add_autoincrement(pParse, fieldno) != 0)
+ return;
+}
columnlist ::= tcons.
columnname(A) ::= nm(A) typedef(Y). {sqlAddColumn(pParse,&A,&Y);}
@@ -299,8 +308,7 @@ ccons ::= NULL onconf(R). {
sql_column_add_nullable_action(pParse, R);
}
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);
@@ -369,8 +377,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 col_list_with_autoinc(X) RP. {
create_index_def_init(&pParse->create_index_def, NULL, &N, X,
SQL_INDEX_TYPE_CONSTRAINT_PK, SORT_ORDER_ASC, false);
sqlAddPrimaryKey(pParse);
@@ -760,6 +767,37 @@ sortlist(A) ::= expr(Y) sortorder(Z). {
sqlExprListSetSortOrder(A,Z);
}
+/**
+ * Non-terminal rule to store a list of columns within PRIMARY KEY
+ * declaration.
+ */
+%type col_list_with_autoinc {ExprList*}
+%destructor col_list_with_autoinc {sql_expr_list_delete(pParse->db, $$);}
+
+col_list_with_autoinc(A) ::= col_list_with_autoinc(A) COMMA expr(Y)
+ autoinc(I). {
+ uint32_t fieldno;
+ if (I == 1) {
+ if (sql_fieldno_by_name(pParse, Y.pExpr, &fieldno) != 0)
+ return;
+ if (sql_add_autoincrement(pParse, fieldno) != 0)
+ return;
+ }
+ A = sql_expr_list_append(pParse->db, A, Y.pExpr);
+}
+
+col_list_with_autoinc(A) ::= expr(Y) autoinc(I). {
+ if (I == 1) {
+ uint32_t fieldno = 0;
+ if (sql_fieldno_by_name(pParse, Y.pExpr, &fieldno) != 0)
+ return;
+ if (sql_add_autoincrement(pParse, fieldno) != 0)
+ return;
+ }
+ /* A-overwrites-Y. */
+ A = sql_expr_list_append(pParse->db, NULL, Y.pExpr);
+}
+
%type sortorder {int}
sortorder(A) ::= ASC. {A = SORT_ORDER_ASC;}
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index 557e415297..df1238b9e2 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -214,6 +214,8 @@ struct create_table_def {
struct rlist new_check;
/** True, if table to be created has AUTOINCREMENT PK. */
bool has_autoinc;
+ /** Id of field with AUTOINCREMENT. */
+ uint32_t autoinc_fieldno;
};
struct create_view_def {
@@ -461,6 +463,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_fieldno = 0;
}
static inline void
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index fbb39878a7..35fc81dfb4 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4419,4 +4419,35 @@ void
vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
const char *idx_name, const char *table_name);
+/**
+ * Add AUTOINCREMENT feature for one of INTEGER or UNSIGNED fields
+ * of PRIMARY KEY.
+ *
+ * @param parse_context Parsing context.
+ * @param fieldno Field number in space format under construction.
+ *
+ * @retval 0 on success.
+ * @retval -1 if table already has declared AUTOINCREMENT feature.
+ */
+int
+sql_add_autoincrement(struct Parse *parse_context, uint32_t fieldno);
+
+/**
+ * Get fieldno by field name. At the moment of forming space format
+ * there's no tuple dictionary, so we can't use hash, in contrast to
+ * tuple_fieldno_by_name(). However, AUTOINCREMENT can occur at most
+ * once in table's definition, so it's not a big deal if we use O(n)
+ * search.
+ *
+ * @param parse_context Parsing context.
+ * @param field_name Expr that contains field name.
+ * @param fieldno[out] Field number in new space format.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
+ uint32_t *fieldno);
+
#endif /* sqlINT_H */
diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua
index 1d0be8aef5..39e47966b1 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(57)
--!./tcltestrunner.lua
-- 2004 November 12
@@ -561,7 +561,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>
})
@@ -814,4 +814,97 @@ test:do_catchsql_test(
-- </autoinc-gh-3670>
})
+--
+-- gh-4217: make sure that AUTOINCREMENT can be used for any
+-- INTEGER field of PRIMARY KEY.
+--
+
+test:do_execsql_test(
+ "autoinc-11.1",
+ [[
+ CREATE TABLE t11_1 (i INT AUTOINCREMENT, a INT, PRIMARY KEY(i, a));
+ INSERT INTO t11_1 VALUES (NULL, 1), (NULL, 1), (NULL, 1);
+ SELECT * FROM t11_1;
+ ]], {
+ 1, 1, 2, 1, 3, 1
+ })
+
+test:do_execsql_test(
+ "autoinc-11.2",
+ [[
+ CREATE TABLE t11_2 (i INT AUTOINCREMENT, a INT, PRIMARY KEY(a, i));
+ INSERT INTO t11_2 VALUES (NULL, 1), (NULL, 1), (NULL, 1);
+ SELECT * FROM t11_2;
+ ]], {
+ 1, 1, 2, 1, 3, 1
+ })
+
+test:do_execsql_test(
+ "autoinc-11.3",
+ [[
+ CREATE TABLE t11_3 (i INT, a INT, PRIMARY KEY(i AUTOINCREMENT, a));
+ INSERT INTO t11_3 VALUES (NULL, 1), (NULL, 1), (NULL, 1);
+ SELECT * FROM t11_3;
+ ]], {
+ 1, 1, 2, 1, 3, 1
+ })
+
+test:do_execsql_test(
+ "autoinc-11.4",
+ [[
+ CREATE TABLE t11_4 (i INT, a INT, PRIMARY KEY(a, i AUTOINCREMENT));
+ INSERT INTO t11_4 VALUES (NULL, 1), (NULL, 1), (NULL, 1);
+ SELECT * FROM t11_4;
+ ]], {
+ 1, 1, 2, 1, 3, 1
+ })
+
+test:do_catchsql_test(
+ "autoinc-11.5",
+ [[
+ CREATE TABLE t11_5 (i INT, a INT, PRIMARY KEY(a, i COLLATE "unicode_ci" AUTOINCREMENT));
+ ]], {
+ 1, "Wrong index options (field 2): collation is reasonable only for string and scalar parts"
+ })
+
+test:do_catchsql_test(
+ "autoinc-11.6",
+ [[
+ CREATE TABLE t11_6 (i INT, a INT, b INT AUTOINCREMENT, PRIMARY KEY(a, i));
+ ]], {
+ 1, "Can't create or modify index 'pk_unnamed_T11_6_1' in space 'T11_6': sequence field must be a part of the index"
+ })
+
+test:do_catchsql_test(
+ "autoinc-11.7",
+ [[
+ CREATE TABLE t11_7 (i INT AUTOINCREMENT, a INT AUTOINCREMENT, PRIMARY KEY(a, i));
+ ]], {
+ 1, "Syntax error in CREATE TABLE: Table must feature at most one AUTOINCREMENT field"
+ })
+
+test:do_catchsql_test(
+ "autoinc-11.8",
+ [[
+ CREATE TABLE t11_8 (i INT, a INT, PRIMARY KEY(a AUTOINCREMENT, i AUTOINCREMENT));
+ ]], {
+ 1, "Syntax error in CREATE TABLE: Table must feature at most one AUTOINCREMENT field"
+ })
+
+test:do_catchsql_test(
+ "autoinc-11.9",
+ [[
+ CREATE TABLE t11_9 (i INT, PRIMARY KEY(a AUTOINCREMENT), a INT);
+ ]], {
+ 1, "Can’t resolve field 'A'"
+ })
+
+test:do_catchsql_test(
+ "autoinc-11.10",
+ [[
+ CREATE TABLE t11_8 (i INT, a INT, PRIMARY KEY(a, 1 AUTOINCREMENT));
+ ]], {
+ 1, "Expressions are prohibited in an index definition"
+})
+
test:finish_test()
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index 05788e156f..cb8952424f 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>
})
diff --git a/test/sql-tap/where4.test.lua b/test/sql-tap/where4.test.lua
index 76e04cb841..e389726660 100755
--- a/test/sql-tap/where4.test.lua
+++ b/test/sql-tap/where4.test.lua
@@ -222,7 +222,7 @@ test:do_execsql_test(
INSERT INTO t2 VALUES(3);
-- Allow the 'x' syntax for backwards compatibility
- CREATE TABLE t4(x INT,y INT,z INT,PRIMARY KEY(x ASC, y ASC));
+ CREATE TABLE t4(x INT,y INT,z INT,PRIMARY KEY(x, y));
SELECT *
FROM t2 LEFT JOIN t4 b1
--
GitLab