[Tarantool-patches] [tarantool-patches] [PATCH v1 1/1] sql: change the way to set AUTOINCREMENT
Nikita Pettik
korablev at tarantool.org
Mon Oct 14 15:28:17 MSK 2019
On 09 Oct 17:30, Mergen Imeev wrote:
> Hi! Thank you for review. My answers, diff and new patch
> below.
Pushed with following code-style changes:
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index edaf5d09c..233f56734 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3221,7 +3221,6 @@ int
sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
uint32_t *fieldno)
{
- uint32_t i;
struct space_def *def = parse_context->create_table_def.new_space->def;
struct Expr *name = sqlExprSkipCollate(field_name);
if (name->op != TK_ID) {
@@ -3229,6 +3228,7 @@ sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
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;
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 639f409ae..ed59a875a 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -230,6 +230,7 @@ columnlist ::= columnlist COMMA columnname carglist autoinc(I). {
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)
@@ -376,7 +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_with_autoinc(X) RP. {
+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);
@@ -767,26 +768,32 @@ sortlist(A) ::= expr(Y) sortorder(Z). {
}
/**
- * The sortlist_with_autoinc rule works in the same way as the
- * sortlist rule, except that it can work with the AUTOINCREMENT
- * keyword and cannot specify the sort order.
+ * Non-terminal rule to store a list of columns within PRIMARY KEY
+ * declaration.
*/
-%type sortlist_with_autoinc {ExprList*}
-%destructor sortlist_with_autoinc {sql_expr_list_delete(pParse->db, $$);}
+%type col_list_with_autoinc {ExprList*}
+%destructor col_list_with_autoinc {sql_expr_list_delete(pParse->db, $$);}
-sortlist_with_autoinc(A) ::= sortlist_with_autoinc(A) COMMA expr(Y)
+col_list_with_autoinc(A) ::= col_list_with_autoinc(A) COMMA expr(Y)
autoinc(I). {
uint32_t fieldno;
- if (I == 1 && (sql_fieldno_by_name(pParse, Y.pExpr, &fieldno) != 0 ||
- sql_add_autoincrement(pParse, fieldno) != 0))
- return;
+ 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);
}
-sortlist_with_autoinc(A) ::= expr(Y) autoinc(I). {
- uint32_t fieldno;
- if (I == 1 && (sql_fieldno_by_name(pParse, Y.pExpr, &fieldno) != 0 ||
- sql_add_autoincrement(pParse, fieldno) != 0))
- return;
+
+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);
}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index de4a96030..35fc81dfb 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4424,16 +4424,20 @@ vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
* of PRIMARY KEY.
*
* @param parse_context Parsing context.
- * @param fieldno Field number in new space format.
+ * @param fieldno Field number in space format under construction.
*
* @retval 0 on success.
- * @retval -1 on error.
+ * @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.
+ * 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.
diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua
index 83d62d723..39e47966b 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(56)
+test:plan(57)
--!./tcltestrunner.lua
-- 2004 November 12
@@ -826,9 +826,7 @@ test:do_execsql_test(
INSERT INTO t11_1 VALUES (NULL, 1), (NULL, 1), (NULL, 1);
SELECT * FROM t11_1;
]], {
- -- <autoinc-11.1>
1, 1, 2, 1, 3, 1
- -- </autoinc-11.1>
})
test:do_execsql_test(
@@ -838,9 +836,7 @@ test:do_execsql_test(
INSERT INTO t11_2 VALUES (NULL, 1), (NULL, 1), (NULL, 1);
SELECT * FROM t11_2;
]], {
- -- <autoinc-11.2>
1, 1, 2, 1, 3, 1
- -- </autoinc-11.2>
})
test:do_execsql_test(
@@ -850,9 +846,7 @@ test:do_execsql_test(
INSERT INTO t11_3 VALUES (NULL, 1), (NULL, 1), (NULL, 1);
SELECT * FROM t11_3;
]], {
- -- <autoinc-11.3>
1, 1, 2, 1, 3, 1
- -- </autoinc-11.3>
})
test:do_execsql_test(
@@ -862,9 +856,7 @@ test:do_execsql_test(
INSERT INTO t11_4 VALUES (NULL, 1), (NULL, 1), (NULL, 1);
SELECT * FROM t11_4;
]], {
- -- <autoinc-11.4>
1, 1, 2, 1, 3, 1
- -- </autoinc-11.4>
})
test:do_catchsql_test(
@@ -872,9 +864,7 @@ test:do_catchsql_test(
[[
CREATE TABLE t11_5 (i INT, a INT, PRIMARY KEY(a, i COLLATE "unicode_ci" AUTOINCREMENT));
]], {
- -- <autoinc-11.5>
1, "Wrong index options (field 2): collation is reasonable only for string and scalar parts"
- -- </autoinc-11.5>
})
test:do_catchsql_test(
@@ -882,9 +872,7 @@ test:do_catchsql_test(
[[
CREATE TABLE t11_6 (i INT, a INT, b INT AUTOINCREMENT, PRIMARY KEY(a, i));
]], {
- -- <autoinc-11.6>
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"
- -- </autoinc-11.6>
})
test:do_catchsql_test(
@@ -892,9 +880,7 @@ test:do_catchsql_test(
[[
CREATE TABLE t11_7 (i INT AUTOINCREMENT, a INT AUTOINCREMENT, PRIMARY KEY(a, i));
]], {
- -- <autoinc-11.7>
1, "Syntax error in CREATE TABLE: Table must feature at most one AUTOINCREMENT field"
- -- </autoinc-11.7>
})
test:do_catchsql_test(
@@ -902,19 +888,23 @@ test:do_catchsql_test(
[[
CREATE TABLE t11_8 (i INT, a INT, PRIMARY KEY(a AUTOINCREMENT, i AUTOINCREMENT));
]], {
- -- <autoinc-11.8>
1, "Syntax error in CREATE TABLE: Table must feature at most one AUTOINCREMENT field"
- -- </autoinc-11.8>
})
test:do_catchsql_test(
- "autoinc-11.8",
+ "autoinc-11.9",
[[
CREATE TABLE t11_9 (i INT, PRIMARY KEY(a AUTOINCREMENT), a INT);
]], {
- -- <autoinc-11.8>
1, "Can’t resolve field 'A'"
- -- </autoinc-11.8>
})
+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"
+})
+
More information about the Tarantool-patches
mailing list