[tarantool-patches] Re: [PATCH v1 1/1] sql: change the way to set AUTOINCREMENT

Mergen Imeev imeevma at tarantool.org
Thu Sep 26 16:20:44 MSK 2019


Hi! Thank you for review. My answers, diff and new patch
below.

On Wed, Sep 25, 2019 at 02:04:46PM +0300, Nikita Pettik wrote:
> On 21 Sep 10:22, Mergen Imeev wrote:
> > Hi! Thank you for review! My answers, diff and bew patch
> > below.
> > 
> > On Wed, Sep 18, 2019 at 08:22:27PM +0300, Nikita Pettik wrote:
> > > On 28 Aug 14:47, Mergen Imeev wrote:
> > > > 
> > > > From f71cec86f8a19c60c2c71f78d25c473e7495b2e3 Mon Sep 17 00:00:00 2001
> > > > From: Mergen Imeev <imeevma at gmail.com>
> > > > Date: Tue, 27 Aug 2019 14:03:58 +0300
> > > > Subject: [PATCH] sql: AUTOINCREMENT for multipart PK
> > > > 
> > > > diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
> > > > index be3c5c3..371da79 100644
> > > > --- a/src/box/sql/parse.y
> > > > +++ b/src/box/sql/parse.y
> > > > @@ -225,8 +225,34 @@ create_table_end ::= . { sqlEndTable(pParse); }
> > > >   */
> > > >  
> > > >  columnlist ::= columnlist COMMA tcons.
> > > > -columnlist ::= columnlist COMMA columnname carglist.
> > > > -columnlist ::= columnname carglist.
> > > > +columnlist ::= columnlist COMMA columnname carglist autoinc(I). {
> > > > +  if (I == 1) {
> > > > +    if (pParse->create_table_def.has_autoinc) {
> > > 
> > > Nit: just squash these two conditions in one diff:
> > > 
> > > if (I && pParse->create_table_def.has_autoinc) ...
> > > 
> > Not sure if that would be better. Moved this code to a new
> > function.
> > 
> > > > +      diag_set(ClientError, ER_SQL_SYNTAX, "CREATE TABLE", "Table must feature "
> > > > +               "at most one AUTOINCREMENT field");
> > > > +      pParse->is_aborted = true;
> > > > +      return;
> > > > +    }
> > > > +    pParse->create_table_def.has_autoinc = true;
> > > > +    pParse->create_table_def.autoinc_field_id =
> > > > +      pParse->create_table_def.new_space->def->field_count - 1;
> > > > +    assert(pParse->create_table_def.autoinc_field_name == NULL);
> > > 
> > > Why not always use field_name? Why not move this logic to autoinc() rule?
> > > This code duplication looks quite awful.
> > > 
> > This would be quite problematic, since we need to get the
> > field name by its identifier, and then get the field
> > identifier by its name. Moved this code to a new function.
> 
> Ok, but then you can do it vice versa: covert name to id right after
> parsing. As far as I remember, constraint definitions always come
> after format declaration. In other words, one can't create table this
> way: CREATE TABLE (a INT, PRIMARY KEY (id), id INT);
> So you can always point out field number using name.
>  
True. Fixed.

> > > > +%type sortlist_with_autoinc {ExprList*}
> > > > +%destructor sortlist_with_autoinc {sql_expr_list_delete(pParse->db, $$);}
> > > > +
> > > > +sortlist_with_autoinc(A) ::= sortlist_with_autoinc(A) COMMA expr(Y) sortorder(Z) autoinc(I). {i
> > > 
> > > Why do you need expr? Expressions are prohibited in index definition.
> > > 
> > I just copied the "sortlist" rule and changed it a bit.
> 
> That's not a reason.
> 
> > Since after processing the AUTOINCREMENT keyword, it should
> > work the same as the "sortlist" rule, I did not change the
> > original arguments of the rule.
> 
> Ok, then fix (in a separate patch) sortlist as well. Or, what is better,
> introduce new rule which would fulfill all requirenments for index
> definition (both for auto-incremented and without auto-increment). 
>  
After some investigation we found out that it would be
quite troublesome to store list of tokens instead of
list of expressions.


Diff:

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index c0ca425..3d642ed 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -937,23 +937,12 @@ 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 space_def *space_def,
 			      struct index_def *idx_def)
 {
 	uint32_t i;
 	char *path = NULL;
 	uint32_t fieldno = pParse->create_table_def.autoinc_field_id;
 
-	char *name = pParse->create_table_def.autoinc_field_name;
-	if (name != NULL) {
-		for (i = 0; i < space_def->field_count; ++i) {
-			if (strcmp(space_def->fields[i].name, name) == 0)
-				break;
-		}
-		assert(i < space_def->field_count);
-		fieldno = i;
-	}
-
 	struct key_part *part = NULL;
 	for (i = 0; i < idx_def->key_def->part_count; ++i) {
 		part = &idx_def->key_def->parts[i];
@@ -1261,11 +1250,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->def,
-						      new_space->index[0]->def);
+		int reg_space_seq_record = emitNewSysSpaceSequenceRecord(pParse,
+							reg_space_id, reg_seq_id,
+							new_space->index[0]->def);
 		sqlVdbeAddOp2(v, OP_SInsert, BOX_SPACE_SEQUENCE_ID,
 			      reg_space_seq_record);
 	}
@@ -3235,8 +3222,7 @@ vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id,
 }
 
 int
-sql_add_autoincrement(struct Parse *parse_context, struct Expr *field_name,
-		      uint32_t field_id)
+sql_add_autoincrement(struct Parse *parse_context, uint32_t field_id)
 {
 	if (parse_context->create_table_def.has_autoinc) {
 		diag_set(ClientError, ER_SQL_SYNTAX, "CREATE TABLE", "Table "
@@ -3245,21 +3231,35 @@ sql_add_autoincrement(struct Parse *parse_context, struct Expr *field_name,
 		return -1;
 	}
 	parse_context->create_table_def.has_autoinc = true;
-	if (field_name != NULL) {
-		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;
-		}
-		char *name_str = region_alloc(&parse_context->region,
-					      strlen(name->u.zToken) + 1);
-		strcpy(name_str, name->u.zToken);
-		parse_context->create_table_def.autoinc_field_name = name_str;
-		return 0;
-	}
 	parse_context->create_table_def.autoinc_field_id = field_id;
-	assert(parse_context->create_table_def.autoinc_field_name == NULL);
+	return 0;
+}
+
+int
+sql_field_id_by_name(struct Parse *parse_context, struct Expr *field_name,
+		     uint32_t *field_id)
+{
+	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) {
+		diag_set(ClientError, ER_INDEX_DEF_UNSUPPORTED, "Expressions");
+		parse_context->is_aborted = true;
+		return -1;
+	}
+	char *name_str = region_alloc(&parse_context->region,
+				      strlen(name->u.zToken) + 1);
+	strcpy(name_str, name->u.zToken);
+	for (i = 0; i < def->field_count; ++i) {
+		if (strcmp(def->fields[i].name, name_str) == 0)
+			break;
+	}
+	/*
+	 * It is possible that i == def->field_count. It means
+	 * that field with AUTOINCREMENT wasn't described before
+	 * PRIMARY KEY() keywords. The error will be thrown later
+	 * in parser.
+	 */
+	*field_id = i;
 	return 0;
 }
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 65a1569..62cbf74 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -227,12 +227,12 @@ create_table_end ::= . { sqlEndTable(pParse); }
 columnlist ::= columnlist COMMA tcons.
 columnlist ::= columnlist COMMA columnname carglist autoinc(I). {
   uint32_t field_id = pParse->create_table_def.new_space->def->field_count - 1;
-  if (I == 1 && sql_add_autoincrement(pParse, NULL, field_id) != 0)
+  if (I == 1 && sql_add_autoincrement(pParse, field_id) != 0)
     return;
 }
 columnlist ::= columnname carglist autoinc(I). {
   uint32_t field_id = pParse->create_table_def.new_space->def->field_count - 1;
-  if (I == 1 && sql_add_autoincrement(pParse, NULL, field_id) != 0)
+  if (I == 1 && sql_add_autoincrement(pParse, field_id) != 0)
     return;
 }
 columnlist ::= tcons.
@@ -775,13 +775,17 @@ sortlist(A) ::= expr(Y) sortorder(Z). {
 
 sortlist_with_autoinc(A) ::= sortlist_with_autoinc(A) COMMA expr(Y) sortorder(Z)
                              autoinc(I). {
-  if (I == 1 && sql_add_autoincrement(pParse, Y.pExpr, 0) != 0)
+  uint32_t field_id;
+  if (I == 1 && (sql_field_id_by_name(pParse, Y.pExpr, &field_id) != 0 ||
+                 sql_add_autoincrement(pParse, field_id) != 0))
     return;
   A = sql_expr_list_append(pParse->db, A, Y.pExpr);
   sqlExprListSetSortOrder(A, Z);
 }
 sortlist_with_autoinc(A) ::= expr(Y) sortorder(Z) autoinc(I). {
-  if (I == 1 && sql_add_autoincrement(pParse, Y.pExpr, 0) != 0)
+  uint32_t field_id;
+  if (I == 1 && (sql_field_id_by_name(pParse, Y.pExpr, &field_id) != 0 ||
+                 sql_add_autoincrement(pParse, field_id) != 0))
     return;
   /* A-overwrites-Y. */
   A = sql_expr_list_append(pParse->db, NULL, Y.pExpr);
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index e4c153a..37deb9e 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -214,16 +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 in case this keyword
-	 * follow definition of the field.
-	 */
+	/** Id of field with AUTOINCREMENT. */
 	uint32_t autoinc_field_id;
-	/**
-	 * Name of field with AUTOINCREMENT in case this keyword
-	 * defined in PK definition.
-	 */
-	char *autoinc_field_name;
 };
 
 struct create_view_def {
@@ -472,7 +464,6 @@ create_table_def_init(struct create_table_def *table_def, struct Token *name,
 	rlist_create(&table_def->new_fkey);
 	rlist_create(&table_def->new_check);
 	table_def->autoinc_field_id = 0;
-	table_def->autoinc_field_name = NULL;
 }
 
 static inline void
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index d7976a3..4421a48 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4434,7 +4434,10 @@ vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
  * @retval -1 on error.
  */
 int
-sql_add_autoincrement(struct Parse *parse_context, struct Expr *field_name,
-		      uint32_t field_id);
+sql_add_autoincrement(struct Parse *parse_context, uint32_t field_id);
+
+int
+sql_field_id_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 de466e6..83d62d7 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(55)
+test:plan(56)
 
 --!./tcltestrunner.lua
 -- 2004 November 12
@@ -907,4 +907,14 @@ test:do_catchsql_test(
         -- </autoinc-11.8>
     })
 
+test:do_catchsql_test(
+    "autoinc-11.8",
+    [[
+        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:finish_test()



New patch:

>From caeda04296ee67a71afed2a1cb97bbf020f85d5b Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma at 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));

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index b4f114e..3d642ed 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;
@@ -945,10 +939,17 @@ static int
 emitNewSysSpaceSequenceRecord(Parse *pParse, int reg_space_id, int reg_seq_id,
 			      struct index_def *idx_def)
 {
-	struct key_part *part = &idx_def->key_def->parts[0];
-	int fieldno = part->fieldno;
+	uint32_t i;
 	char *path = NULL;
-	if (part->path != NULL) {
+	uint32_t fieldno = pParse->create_table_def.autoinc_field_id;
+
+	struct key_part *part = NULL;
+	for (i = 0; i < idx_def->key_def->part_count; ++i) {
+		part = &idx_def->key_def->parts[i];
+		if (part->fieldno == fieldno)
+			break;
+	}
+	if (i != idx_def->key_def->part_count && part->path != NULL) {
 		path = sqlDbStrNDup(pParse->db, part->path, part->path_len);
 		if (path == NULL)
 			return -1;
@@ -3219,3 +3220,46 @@ 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 field_id)
+{
+	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_field_id = field_id;
+	return 0;
+}
+
+int
+sql_field_id_by_name(struct Parse *parse_context, struct Expr *field_name,
+		     uint32_t *field_id)
+{
+	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) {
+		diag_set(ClientError, ER_INDEX_DEF_UNSUPPORTED, "Expressions");
+		parse_context->is_aborted = true;
+		return -1;
+	}
+	char *name_str = region_alloc(&parse_context->region,
+				      strlen(name->u.zToken) + 1);
+	strcpy(name_str, name->u.zToken);
+	for (i = 0; i < def->field_count; ++i) {
+		if (strcmp(def->fields[i].name, name_str) == 0)
+			break;
+	}
+	/*
+	 * It is possible that i == def->field_count. It means
+	 * that field with AUTOINCREMENT wasn't described before
+	 * PRIMARY KEY() keywords. The error will be thrown later
+	 * in parser.
+	 */
+	*field_id = i;
+	return 0;
+}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 643e025..62cbf74 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -225,8 +225,16 @@ 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 field_id = pParse->create_table_def.new_space->def->field_count - 1;
+  if (I == 1 && sql_add_autoincrement(pParse, field_id) != 0)
+    return;
+}
+columnlist ::= columnname carglist autoinc(I). {
+  uint32_t field_id = pParse->create_table_def.new_space->def->field_count - 1;
+  if (I == 1 && sql_add_autoincrement(pParse, field_id) != 0)
+    return;
+}
 columnlist ::= tcons.
 columnname(A) ::= nm(A) typedef(Y). {sqlAddColumn(pParse,&A,&Y);}
 
@@ -299,8 +307,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 +376,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_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 +766,32 @@ sortlist(A) ::= expr(Y) sortorder(Z). {
   sqlExprListSetSortOrder(A,Z);
 }
 
+/**
+ * The sortlist_with_autoinc rule works the same as the sortlist
+ * rule, except that it can work with the AUTOINCREMENT keyword.
+ */
+%type sortlist_with_autoinc {ExprList*}
+%destructor sortlist_with_autoinc {sql_expr_list_delete(pParse->db, $$);}
+
+sortlist_with_autoinc(A) ::= sortlist_with_autoinc(A) COMMA expr(Y) sortorder(Z)
+                             autoinc(I). {
+  uint32_t field_id;
+  if (I == 1 && (sql_field_id_by_name(pParse, Y.pExpr, &field_id) != 0 ||
+                 sql_add_autoincrement(pParse, field_id) != 0))
+    return;
+  A = sql_expr_list_append(pParse->db, A, Y.pExpr);
+  sqlExprListSetSortOrder(A, Z);
+}
+sortlist_with_autoinc(A) ::= expr(Y) sortorder(Z) autoinc(I). {
+  uint32_t field_id;
+  if (I == 1 && (sql_field_id_by_name(pParse, Y.pExpr, &field_id) != 0 ||
+                 sql_add_autoincrement(pParse, field_id) != 0))
+    return;
+  /* A-overwrites-Y. */
+  A = sql_expr_list_append(pParse->db, NULL, Y.pExpr);
+  sqlExprListSetSortOrder(A, Z);
+}
+
 %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 557e415..37deb9e 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_field_id;
 };
 
 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_field_id = 0;
 }
 
 static inline void
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index fbb3987..4421a48 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4419,4 +4419,25 @@ 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. If field_name is NULL, the field is identified
+ * by its identifier (field_id). Otherwise, field_id is ignored,
+ * and the field is identified by the name of the field that is
+ * contained in field_name.
+ *
+ * @param parse_context Parsing context.
+ * @param field_name Expr that contains field name.
+ * @param field_id Field identifier.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+sql_add_autoincrement(struct Parse *parse_context, uint32_t field_id);
+
+int
+sql_field_id_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 1d0be8a..83d62d7 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(56)
 
 --!./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,107 @@ 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;
+    ]], {
+        -- <autoinc-11.1>
+        1, 1, 2, 1, 3, 1
+        -- </autoinc-11.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;
+    ]], {
+        -- <autoinc-11.2>
+        1, 1, 2, 1, 3, 1
+        -- </autoinc-11.2>
+    })
+
+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;
+    ]], {
+        -- <autoinc-11.3>
+        1, 1, 2, 1, 3, 1
+        -- </autoinc-11.3>
+    })
+
+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;
+    ]], {
+        -- <autoinc-11.4>
+        1, 1, 2, 1, 3, 1
+        -- </autoinc-11.4>
+    })
+
+test:do_catchsql_test(
+    "autoinc-11.5",
+    [[
+        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(
+    "autoinc-11.6",
+    [[
+        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(
+    "autoinc-11.7",
+    [[
+        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(
+    "autoinc-11.8",
+    [[
+        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",
+    [[
+        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:finish_test()
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index 05788e1..cb89524 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>
 	})
 





More information about the Tarantool-patches mailing list