[tarantool-patches] Re: [PATCH v11] sql: add index_def to struct Index

Ivan Koptelov ivan.koptelov at tarantool.org
Thu Jul 5 17:50:41 MSK 2018


> Please, follow our SOP convention: if you send next patch version,
> then resend it as a separate patch (git format-patch -1 --subject-prefix='PATCH v2’),
> and add changelog. If you simply want to send diff, then add it to each pace of
> provided changes. In your case, it is better to send only diff AFTER changes
> (not the whole patch), it makes review process MUCH easier.
Sorry. Would send only diff this time.
>>> Then:
>>>
>>> CREATE TABLE t11 (s1 INT, a, constraint c1 UNIQUE(s1) on conflict replace, PRIMARY KEY(s1));
>>>
>>> In this case creation of unique constraint c1 is omitted, but no errors or warnings are shown.
>>> It is not a problem now, but when ALTER TABLE DROP CONSTRAINT is implemented,
>>> it will be possible to drop c1 constraint. Eventually, user would be disappointed if tried to drop
>>> this constraint but got an error.
>> It seems to be out of the scope of the patch. Appropriate ticket:
>> https://github.com/tarantool/tarantool/issues/3498
> I don’t understand how that ticket is related to given issue. Again, problem lies in
> silent absorption of unique constraint by primary index.
I am not sure what to do with this. In Postgresql these two commands 
work silently, no UNIQUE
constraint is created and ALTER TABLE works silently and do nothing. In 
MySQL both constraints
will be created with no warnings or errors. What do you propose to do?

>
>>>> +struct Index *
>>>> +sql_index_alloc(struct sqlite3 *db, uint32_t part_count)
>>>> {
>>>> -	Index *p;		/* Allocated index object */
>>>> -	int nByte;		/* Bytes of space for Index object + arrays */
>>>> -
>>>> -	nByte = ROUND8(sizeof(Index)) +		    /* Index structure   */
>>>> -	    ROUND8(sizeof(struct coll *) * nCol) +  /* Index.coll_array  */
>>>> -	    ROUND8(sizeof(uint32_t) * nCol) +       /* Index.coll_id_array*/
>>>> -	    ROUND8(sizeof(LogEst) * (nCol + 1) +    /* Index.aiRowLogEst */
>>>> -		   sizeof(i16) * nCol +		    /* Index.aiColumn    */
>>>> -		   sizeof(enum sort_order) * nCol); /* Index.sort_order  */
>>>> -	p = sqlite3DbMallocZero(db, nByte + nExtra);
>>>> -	if (p) {
>>>> -		char *pExtra = ((char *)p) + ROUND8(sizeof(Index));
>>>> -		p->coll_array = (struct coll **)pExtra;
>>>> -		pExtra += ROUND8(sizeof(struct coll **) * nCol);
>>>> -		p->coll_id_array = (uint32_t *) pExtra;
>>>> -		pExtra += ROUND8(sizeof(uint32_t) * nCol);
>>>> -		p->aiRowLogEst = (LogEst *) pExtra;
>>>> -		pExtra += sizeof(LogEst) * (nCol + 1);
>>>> -		p->aiColumn = (i16 *) pExtra;
>>>> -		pExtra += sizeof(i16) * nCol;
>>>> -		p->sort_order = (enum sort_order *) pExtra;
>>>> -		p->nColumn = nCol;
>>>> -		*ppExtra = ((char *)p) + nByte;
>>>> -	}
>>>> +	/* Size of struct Index and aiRowLogEst. */
>>>> +	int nByte = ROUND8(sizeof(struct Index)) +
>>>> +		    ROUND8(sizeof(LogEst) * (part_count + 1));
>>> Do we really need this alignment?
>> No. Removed.
> No, you don’t. Apply this diff:
>
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index d66777f73..d2fed97eb 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -2430,13 +2430,14 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage)
>   }
>   
>   struct Index *
> -sql_index_alloc(struct sqlite3 *db)
> +sql_index_alloc(struct sqlite3 *db, uint32_t part_count)
>   {
>          /* Size of struct Index and aiRowLogEst. */
> -       int index_size = ROUND8(sizeof(struct Index));
> +       int index_size = sizeof(struct Index) +
> +                        sizeof(LogEst) * (part_count + 1);
>          struct Index *p = sqlite3DbMallocZero(db, index_size);
>          if (p != NULL)
> -               p->aiRowLogEst = (LogEst *) ((char *)p + ROUND8(sizeof(*p)));
> +               p->aiRowLogEst = (LogEst *) ((char *)p + sizeof(*p));
>          return p;
>   }
>
> @@ -2769,11 +2757,10 @@ sql_create_index(struct Parse *parse, struct Token *token,
>          if (sqlite3CheckIdentifierName(parse, name) != SQLITE_OK)
>                  goto exit_create_index;
>   
> -       index = sql_index_alloc(db);
> +       index = sql_index_alloc(db, col_list->nExpr);
>
> @@ -2769,11 +2757,10 @@ sql_create_index(struct Parse *parse, struct Token *token,
>          if (sqlite3CheckIdentifierName(parse, name) != SQLITE_OK)
>                  goto exit_create_index;
>   
> -       index = sql_index_alloc(db);
> +       index = sql_index_alloc(db, col_list->nExpr);
>          if (index == NULL)
>                  goto exit_create_index;
>   
> -       assert(EIGHT_BYTE_ALIGNMENT(index->aiRowLogEst));
>
> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> index ae31dfae5..2082d6ca9 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h
> @@ -3631,7 +3631,7 @@ void sqlite3SrcListDelete(sqlite3 *, SrcList *);
>    * @retval not NULL Index object.
>    */
>   struct Index *
> -sql_index_alloc(struct sqlite3 *db);
> +sql_index_alloc(struct sqlite3 *db, uint32_t part_count);
Why do we need to allocate 'part_count' part of memory? It does not seem
to be used for anything.
>
>> Ok, removed detailed description, left a short one.
> @@ -2538,19 +2539,15 @@ addIndexToTable(Index * pIndex, Table * pTab)
>    * @param expr_list List of expressions, describe which columns
>    *                  of 'table' are used in index and also their
>    *                  collations, orders, etc.
> - * @param idx_type Index type, one of the following:
> - *                 SQLITE_IDXTYPE_APPDEF     0
> - *                 SQLITE_IDXTYPE_UNIQUE     1
> - *                 SQLITE_IDXTYPE_PRIMARYKEY 2.
> + * @param idx_type Index type: UNIQUE constraint, PK constraint,
> + *                 or created by <CREATE INDEX ...> stmt.
Ok.
>>>> @@ -2731,42 +2765,38 @@ sql_create_index(struct Parse *parse, struct Token *token,
>>>> 	 * primary key or UNIQUE constraint.  We have to invent
>>>> 	 * our own name.
>>>> 	 */
>>>> -	if (token) {
>>>> -		zName = sqlite3NameFromToken(db, token);
>>>> -		if (zName == 0)
>>>> +	if (token != NULL) {
>>>> +		name = sqlite3NameFromToken(db, token);
>>>> +		if (name == NULL)
>>>> 			goto exit_create_index;
>>>> -		assert(token->z != 0);
>>>> +		assert(token->z != NULL);
>>>> 		if (!db->init.busy) {
>>>> -			if (sqlite3HashFind(&db->pSchema->tblHash, zName) !=
>>>> +			if (sqlite3HashFind(&db->pSchema->tblHash, name) !=
>>>> 			    NULL) {
>>>> -				sqlite3ErrorMsg(parse,
>>>> -						"there is already a table named %s",
>>>> -						zName);
>>>> +				sqlite3ErrorMsg(parse, "there is already a "\
>>>> +						"table named %s", name);
>>>> 				goto exit_create_index;
>>>> 			}
>>>> 		}
> You forgot about my comment:
>
>> There is no need to prohibit creating index with table name, since
>> index name is local for given table. And most of other DBs also
>> allow to create index with table name.
> Apply diff:
>
> @@ -2705,14 +2702,6 @@ sql_create_index(struct Parse *parse, struct Token *token,
>                  if (name == NULL)
>                          goto exit_create_index;
>                  assert(token->z != NULL);
> -               if (!db->init.busy) {
> -                       if (sqlite3HashFind(&db->pSchema->tblHash, name) !=
> -                           NULL) {
> -                               sqlite3ErrorMsg(parse, "there is already a "\
> -                                               "table named %s", name);
> -                               goto exit_create_index;
> -                       }
> -               }
Ok.
>>>> +
>>>> +	bool is_unique = on_error != ON_CONFLICT_ACTION_NONE;
>>> It seems to be so messy defining uniqueness by ON_CONFLICT_ACTION.
>>> Lets refactor it somehow.
>> Not sure about this. It seems that information about uniqueness is
>> only in on_error parameter.
> What? Lets then change signature and from parser pass additional parameter
> which would tell if index is unique or not.
Ok, done. Also perform small refactoring in parser.
>
>>>> diff --git a/src/box/sql/where.c b/src/box/sql/where.c
>>>> index 85143ed20..7ca02095f 100644
>>>> --- a/src/box/sql/where.c
>>>> +++ b/src/box/sql/where.c
>>>> @@ -372,13 +372,19 @@ whereScanInit(WhereScan * pScan,	/* The WhereScan object being initialized */
>>>> 	pScan->is_column_seen = false;
>>>> 	if (pIdx) {
>>>> 		int j = iColumn;
>>>> -		iColumn = pIdx->aiColumn[j];
>>>> +		iColumn = pIdx->def->key_def->parts[j].fieldno;
>>>> +		/*
>>>> +		 * pIdx->tnum == 0 means that pIdx is a fake
>>>> +		 * integer primary key index.
>>>> +		 */
>>>> +		if (pIdx->tnum == 0)
>>>> +			iColumn = -1;
>>> We are going to remove tnum from struct Index and struct Table.
>>> So, if it is possible, use index->def->iid instead (or smth else).
>> Removed with ‘fake_autoindex'
> Well, if smb called index ‘fake_autoindex’, I guess strange things would happen.
> Could we use for instance def->space_id == 0 as a sign of ‘fake_index'?.
Yes, it seems to be a better solution. What do you think about adding a 
field to
index_def.opts - index_def.opts.is_fake_pk?
>
>> src/box/errcode.h                                  |   1 +
>> src/box/sql.c                                      |  54 +-
>> src/box/sql/analyze.c                              |  85 +--
>> src/box/sql/build.c                                | 816 ++++++++++-----------
>> src/box/sql/delete.c                               |  10 +-
>> src/box/sql/expr.c                                 |  61 +-
>> src/box/sql/fkey.c                                 | 216 +++---
>> src/box/sql/insert.c                               | 145 ++--
>> src/box/sql/pragma.c                               |  30 +-
>> src/box/sql/select.c                               |   2 +-
>> src/box/sql/sqliteInt.h                            | 116 +--
>> src/box/sql/update.c                               |  39 +-
>> src/box/sql/vdbeaux.c                              |   2 +-
>> src/box/sql/vdbemem.c                              |  21 +-
>> src/box/sql/where.c                                | 192 ++---
>> src/box/sql/wherecode.c                            | 102 +--
>> test/box/misc.result                               |   1 +
>> test/sql-tap/analyze6.test.lua                     |   6 +-
>> .../{collation.test.lua => collation1.test.lua}    |   7 +-
>> test/sql-tap/colname.test.lua                      |   4 +-
>> test/sql-tap/gh-2931-savepoints.test.lua           |   2 +-
>> test/sql-tap/gh2140-trans.test.lua                 |   2 +-
>> test/sql-tap/gh2259-in-stmt-trans.test.lua         |   8 +-
>> test/sql-tap/gh2964-abort.test.lua                 |   2 +-
>> test/sql-tap/identifier-characters.test.lua        |   2 +-
>> test/sql-tap/identifier_case.test.lua              |   4 +-
>> test/sql-tap/index1.test.lua                       |  14 +-
>> test/sql-tap/index7.test.lua                       |  21 +-
>> test/sql-tap/intpkey.test.lua                      |   4 +-
>> test/sql-tap/misc1.test.lua                        |   2 +-
>> test/sql-tap/unique.test.lua                       |   8 +-
>> test/sql-tap/update.test.lua                       |   6 +-
>> test/sql/insert-unique.result                      |   3 +-
>> test/sql/iproto.result                             |   2 +-
>> test/sql/message-func-indexes.result               |   8 +-
>> test/sql/on-conflict.result                        |   2 +-
>> test/sql/persistency.result                        |   6 +-
>> test/sql/transition.result                         |   6 +-
>> 38 files changed, 965 insertions(+), 1047 deletions(-)
>> rename test/sql-tap/{collation.test.lua => collation1.test.lua} (97%)
> Why have you renamed this test?
Renamed back.
>
>> +	if (is_system_space && idx_type == SQLITE_IDXTYPE_APPDEF) {
>> +		diag_set(ClientError, ER_MODIFY_INDEX, name,
>> +			 table->def->name, "creating indexes on system "
>> +					   "spaces are prohibited”);
> +               diag_set(ClientError, ER_MODIFY_INDEX, name, table->def->name,
> +                        "can't create index on system space");
Ok.
>> +	char *sql_stmt = "";
>> +	if (!db->init.busy && tbl_name != NULL) {
>> +		int n = (int) (parse->sLastToken.z - token->z) +
>> +			parse->sLastToken.n;
>> +		if (token->z[n - 1] == ';')
>> +			n--;
>> +		sql_stmt = sqlite3MPrintf(db, "CREATE%s INDEX %.*s",
>> +				       on_error == ON_CONFLICT_ACTION_NONE ?
>> +				       "" : " UNIQUE", n, token->z);
> Wrong alignment:
>
> @@ -2809,8 +2796,8 @@ sql_create_index(struct Parse *parse, struct Token *token,
>                  if (token->z[n - 1] == ';')
>                          n--;
>                  sql_stmt = sqlite3MPrintf(db, "CREATE%s INDEX %.*s",
> -                                      on_error == ON_CONFLICT_ACTION_NONE ?
> -                                      "" : " UNIQUE", n, token->z);
> +                                         on_error == ON_CONFLICT_ACTION_NONE ?
> +                                         "" : " UNIQUE", n, token->z);
Sorry, fixed.
> Moreover, sql_stmt is obtained from sqlite3Malloc() and assigned to index->opts,
> which in turn is released by common free().
In index_def_new() (where we pass opts) given sql_stmt is copied using 
strdup, so
it's okay to free() it, isn't it?
>
>> diff --git a/test/sql-tap/index7.test.lua b/test/sql-tap/index7.test.lua
>> index 336f42796..4bd01b8b3 100755
>> --- a/test/sql-tap/index7.test.lua
>> +++ b/test/sql-tap/index7.test.lua
>> @@ -1,6 +1,6 @@
>> #!/usr/bin/env tarantool
>> test = require("sqltester")
>> -test:plan(5)
>> +test:plan(7)
>>   --!./tcltestrunner.lua
>> -- 2013-11-04
>> @@ -48,7 +48,7 @@ end
>> -- do_test index7-1.1a {
>> --   capture_pragma db out {PRAGMA index_list(t1)}
>> --   db eval {SELECT "name", "partial", '|' FROM out ORDER BY "name"}
>> --- } {sqlite_autoindex_t1_1 0 | t1a 1 | t1b 1 |}
>> +-- } {sql_autoindex_t1_1 0 | t1a 1 | t1b 1 |}
>> -- # Make sure the count(*) optimization works correctly with
>> -- # partial indices.  Ticket [a5c8ed66cae16243be6] 2013-10-03.
>> -- #
>> @@ -303,4 +303,21 @@ test:do_catchsql_test(
>>          1, "keyword \"WHERE\" is reserved"
>>      })
>> +test:do_catchsql_test(
>> +        "index7-6.6",
>> +        'CREATE TABLE test2 (a int, b int, c int, PRIMARY KEY (a, a, a, b, b, a, c))',
>> +        nil)
> Why nil? What does this test check at all?
The idea was to check that removing duplicates works fine. Fixed the 
test to check names of
resulting pk columns with expected.
>
>> +
>> +test:do_catchsql_test(
>> +        "index7-6.7",
>> +        [[
>> +            CREATE TABLE test4(a,b,c,d, PRIMARY KEY(a,a,a,b,c));
>> +            CREATE INDEX index1 on test4(b,c,a,c);
>> +            SELECT "_index"."name" FROM "_index" JOIN "_space" WHERE
>> +            "_index"."id" = "_space"."id" AND
>> +            "_space"."name"='TEST4' AND
>> +            "_index"."name"='INDEX1’;
> Use indentation for nested select clause to make it more readable.
> Moreover, add comment to the test which would explain what exactly this test checks.
Ok, done.

Here is diff:
--

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index d66777f73..0cb0b8e08 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -984,8 +984,10 @@ sqlite3AddPrimaryKey(Parse * pParse,	/* Parsing context */
  		sqlite3ErrorMsg(pParse, "AUTOINCREMENT is only allowed on an "
  				"INTEGER PRIMARY KEY or INT PRIMARY KEY");
  	} else {
+		/* Since this index implements PK, it is unique. */
  		sql_create_index(pParse, 0, 0, pList, onError, 0,
-				 0, sortOrder, false, SQLITE_IDXTYPE_PRIMARYKEY);
+				 0, sortOrder, false, SQLITE_IDXTYPE_PRIMARYKEY,
+				 true);
  		pList = 0;
  	}
  
@@ -1311,9 +1313,10 @@ convertToWithoutRowidTable(Parse * pParse, Table * pTab)
  			return;
  		pList->a[0].sort_order = pParse->iPkSortOrder;
  		assert(pParse->pNewTable == pTab);
+		/* Since this index implements PK, it is unique. */
  		sql_create_index(pParse, 0, 0, pList, pTab->keyConf, 0, 0,
  				 SORT_ORDER_ASC, false,
-				 SQLITE_IDXTYPE_PRIMARYKEY);
+				 SQLITE_IDXTYPE_PRIMARYKEY, true);
  		if (db->mallocFailed)
  			return;
  		pPk = sqlite3PrimaryKeyIndex(pTab);
@@ -2538,10 +2541,8 @@ addIndexToTable(Index * pIndex, Table * pTab)
   * @param expr_list List of expressions, describe which columns
   *                  of 'table' are used in index and also their
   *                  collations, orders, etc.
- * @param idx_type Index type, one of the following:
- *                 SQLITE_IDXTYPE_APPDEF     0
- *                 SQLITE_IDXTYPE_UNIQUE     1
- *                 SQLITE_IDXTYPE_PRIMARYKEY 2.
+ * @param idx_type Index type: UNIQUE constraint, PK constraint,
+ *                 or created by <CREATE INDEX ...> stmt.
   * @param sql_stmt SQL statement, which creates the index.
   * @retval 0 Success.
   * @retval -1 Error.
@@ -2632,7 +2633,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
  		 struct SrcList *tbl_name, struct ExprList *col_list,
  		 enum on_conflict_action on_error, struct Token *start,
  		 struct Expr *where, enum sort_order sort_order,
-		 bool if_not_exist, u8 idx_type)
+		 bool if_not_exist, u8 idx_type, bool is_unique)
  {
  	/* The index to be created. */
  	struct Index *index = NULL;
@@ -2705,14 +2706,6 @@ sql_create_index(struct Parse *parse, struct Token *token,
  		if (name == NULL)
  			goto exit_create_index;
  		assert(token->z != NULL);
-		if (!db->init.busy) {
-			if (sqlite3HashFind(&db->pSchema->tblHash, name) !=
-			    NULL) {
-				sqlite3ErrorMsg(parse, "there is already a "\
-						"table named %s", name);
-				goto exit_create_index;
-			}
-		}
  		if (sqlite3HashFind(&table->idxHash, name) != NULL) {
  			if (!if_not_exist) {
  				sqlite3ErrorMsg(parse,
@@ -2737,9 +2730,8 @@ sql_create_index(struct Parse *parse, struct Token *token,
  	bool is_system_space = BOX_SYSTEM_ID_MIN < table->def->id &&
  			       table->def->id < BOX_SYSTEM_ID_MAX;
  	if (is_system_space && idx_type == SQLITE_IDXTYPE_APPDEF) {
-		diag_set(ClientError, ER_MODIFY_INDEX, name,
-			 table->def->name, "creating indexes on system "
-					   "spaces are prohibited");
+		diag_set(ClientError, ER_MODIFY_INDEX, name, table->def->name,
+			 "can't create index on system space");
  		parse->nErr++;
  		parse->rc = SQL_TARANTOOL_ERROR;
  		goto exit_create_index;
@@ -2809,8 +2801,8 @@ sql_create_index(struct Parse *parse, struct Token *token,
  		if (token->z[n - 1] == ';')
  			n--;
  		sql_stmt = sqlite3MPrintf(db, "CREATE%s INDEX %.*s",
-				       on_error == ON_CONFLICT_ACTION_NONE ?
-				       "" : " UNIQUE", n, token->z);
+					  on_error == ON_CONFLICT_ACTION_NONE ?
+					  "" : " UNIQUE", n, token->z);
  		if (db->mallocFailed || sql_stmt == NULL)
  			goto exit_create_index;
  	}
@@ -2820,9 +2812,8 @@ sql_create_index(struct Parse *parse, struct Token *token,
  	if (db->init.busy)
  		iid = SQLITE_PAGENO_TO_INDEXID(db->init.newTnum);
  
-	bool is_unique = on_error != ON_CONFLICT_ACTION_NONE;
  	if (index_fill_def(parse, index, table, iid, name, strlen(name),
-			   is_unique, col_list, idx_type, sql_stmt) != 0)
+			   is_unique != 0, col_list, idx_type, sql_stmt) != 0)
  		goto exit_create_index;
  	/*
  	 * Remove all redundant columns from the PRIMARY KEY.
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index b2940b7c4..ed3d82f37 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -282,7 +282,7 @@ ccons ::= PRIMARY KEY sortorder(Z) onconf(R) autoinc(I).
                                   {sqlite3AddPrimaryKey(pParse,0,R,I,Z);}
  ccons ::= UNIQUE onconf(R).      {sql_create_index(pParse,0,0,0,R,0,0,
  						   SORT_ORDER_ASC, false,
-						   SQLITE_IDXTYPE_UNIQUE);}
+						   SQLITE_IDXTYPE_UNIQUE, true);}
  ccons ::= CHECK LP expr(X) RP.   {sql_add_check_constraint(pParse,&X);}
  ccons ::= REFERENCES nm(T) eidlist_opt(TA) refargs(R).
                                   {sqlite3CreateForeignKey(pParse,0,&T,TA,R);}
@@ -333,7 +333,8 @@ tcons ::= PRIMARY KEY LP sortlist(X) autoinc(I) RP onconf(R).
  tcons ::= UNIQUE LP sortlist(X) RP onconf(R).
                                   {sql_create_index(pParse,0,0,X,R,0,0,
  						   SORT_ORDER_ASC,false,
-						   SQLITE_IDXTYPE_UNIQUE);}
+						   SQLITE_IDXTYPE_UNIQUE,
+						   true);}
  tcons ::= CHECK LP expr(E) RP onconf.
                                   {sql_add_check_constraint(pParse,&E);}
  tcons ::= FOREIGN KEY LP eidlist(FA) RP
@@ -1194,13 +1195,16 @@ paren_exprlist(A) ::= LP exprlist(X) RP.  {A = X;}
  //
  cmd ::= createkw(S) uniqueflag(U) INDEX ifnotexists(NE) nm(X)
          ON nm(Y) LP sortlist(Z) RP. {
-  sql_create_index(pParse, &X, sqlite3SrcListAppend(pParse->db,0,&Y), Z, U, &S,
-                   NULL, SORT_ORDER_ASC, NE, SQLITE_IDXTYPE_APPDEF);
+  enum on_conflict_action on_error =
+          U ? ON_CONFLICT_ACTION_ABORT : ON_CONFLICT_ACTION_NONE;
+  sql_create_index(pParse, &X, sqlite3SrcListAppend(pParse->db,0,&Y), Z,
+                   on_error, &S, NULL, SORT_ORDER_ASC, NE,
+                   SQLITE_IDXTYPE_APPDEF, U);
  }
  
  %type uniqueflag {int}
-uniqueflag(A) ::= UNIQUE.  {A = ON_CONFLICT_ACTION_ABORT;}
-uniqueflag(A) ::= .        {A = ON_CONFLICT_ACTION_NONE;}
+uniqueflag(A) ::= UNIQUE.  {A = true;}
+uniqueflag(A) ::= .        {A = false;}
  
  
  // The eidlist non-terminal (Expression Id List) generates an ExprList
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index ae31dfae5..e3d568153 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3657,13 +3657,14 @@ sql_index_alloc(struct sqlite3 *db);
   * @param sort_order Sort order of primary key when pList==NULL.
   * @param if_not_exist Omit error if index already exists.
   * @param idx_type The index type.
+ * @param is_unique Is index unique or not.
   */
  void
  sql_create_index(struct Parse *parse, struct Token *token,
  		 struct SrcList *tbl_name, struct ExprList *col_list,
  		 enum on_conflict_action on_error, struct Token *start,
  		 struct Expr *pi_where, enum sort_order sort_order,
-		 bool if_not_exist, u8 idx_type);
+		 bool if_not_exist, u8 idx_type, bool is_unique);
  
  /**
   * This routine will drop an existing named index.  This routine
diff --git a/test/sql-tap/collation1.test.lua b/test/sql-tap/collation.test.lua
similarity index 100%
rename from test/sql-tap/collation1.test.lua
rename to test/sql-tap/collation.test.lua
diff --git a/test/sql-tap/index1.test.lua b/test/sql-tap/index1.test.lua
index a3405a3e7..9ee40fa13 100755
--- a/test/sql-tap/index1.test.lua
+++ b/test/sql-tap/index1.test.lua
@@ -1,6 +1,6 @@
  #!/usr/bin/env tarantool
  test = require("sqltester")
-test:plan(79)
+test:plan(78)
  
  --!./tcltestrunner.lua
  -- 2001 September 15
@@ -374,24 +374,14 @@ test:do_catchsql_test(
          -- </index-6.1c>
      })
  
-test:do_catchsql_test(
-    "index-6.2",
-    [[
-        CREATE INDEX test1 ON test2(g1)
-    ]], {
-        -- <index-6.2>
-        1, "there is already a table named TEST1"
-        -- </index-6.2>
-    })
-
  test:do_execsql_test(
-    "index-6.2b",
+    "index-6.2",
      [[
          SELECT "name" FROM "_index" WHERE "name"='INDEX1' union SELECT "name" FROM "_space" WHERE "name"='TEST1' OR "name"='TEST2'
      ]], {
-        -- <index-6.2b>
+        -- <index-6.2>
          "INDEX1", "TEST1", "TEST2"
-        -- </index-6.2b>
+        -- </index-6.2>
      })
  
  test:do_test(
diff --git a/test/sql-tap/index7.test.lua b/test/sql-tap/index7.test.lua
index 4bd01b8b3..29eca386b 100755
--- a/test/sql-tap/index7.test.lua
+++ b/test/sql-tap/index7.test.lua
@@ -303,20 +303,42 @@ test:do_catchsql_test(
          1, "keyword \"WHERE\" is reserved"
      })
  
+-- Currently, when a user tries to create index (or primary key,
+-- since we implement them as indexes underhood) with duplicated
+-- fields (like 'CREATE INDEX i1 ON t(a, a, a, a, b, c, b)')
+-- tarantool would silently remove duplicated fields and
+-- execute 'CREATE INDEX i1 ON t(a, b, c)'.
+-- This test checks that duplicates are removed correctly.
+--
  test:do_catchsql_test(
-        "index7-6.6",
-        'CREATE TABLE test2 (a int, b int, c int, PRIMARY KEY (a, a, a, b, b, a, c))',
-        nil)
+        "index7-8.1",
+        [[
+            CREATE TABLE t(a,b,c, PRIMARY KEY(a));
+            CREATE INDEX i1 ON t(a, a, b, c, c, b, b, b, c, b, c);
+            pragma index_info = t.i1;
+        ]],
+        {0, {0,0,"A",1,1,"B",2,2,"C"}}
+)
  
+-- There was the following bug:
+-- > CREATE TABLE t1(a,b,c,d, PRIMARY KEY(a,a,a,b,c));
+-- ...
+-- > CREATE INDEX i1 ON t1(b,c,a,c)
+-- ...
+-- But index 'i1' was not actually created and no error was raised.
+-- This test checks that this does not happen anymore (and index is
+-- created successfully).
+--
  test:do_catchsql_test(
-        "index7-6.7",
+        "index7-8.2",
          [[
              CREATE TABLE test4(a,b,c,d, PRIMARY KEY(a,a,a,b,c));
              CREATE INDEX index1 on test4(b,c,a,c);
-            SELECT "_index"."name" FROM "_index" JOIN "_space" WHERE
-            "_index"."id" = "_space"."id" AND
-            "_space"."name"='TEST4' AND
-            "_index"."name"='INDEX1';
+            SELECT "_index"."name"
+            FROM "_index" JOIN "_space" WHERE
+                "_index"."id" = "_space"."id" AND
+                "_space"."name"='TEST4'       AND
+                "_index"."name"='INDEX1';
          ]],
          {0, {'INDEX1'}})
  
-- 





More information about the Tarantool-patches mailing list