, tarantool-patches@freelists.org Thank you for the review!

+	/*
+	 * Here we handle cases like
+	 * CREATE TABLE t1(a UNIQUE, PRIMARY KEY(a))
+	 * In these cases (where UNIQUE constraints precede
+	 * PRIMARY constraint) appropriate Index->def->cmp_def
+	 * can not be set 'on the fly' for indexes implementing
+	 * UNIQUE constraints as PK key_def is missing and
+	 * needed for it.
+	 * At this point we can set appropriate cmp_def, since
+	 * all indexes (from CREATE TABLE statement) is created.
+	 */
+	if (!p->def->opts.is_view) {
+		assert(p->pIndex != NULL);
+		struct Index *pk;
+		for (struct Index *idx = p->pIndex; idx != NULL;
+		     idx = idx->pNext) {
+			if (idx->index_type == SQL_INDEX_TYPE_CONSTRAINT_PK)
+				pk = idx;
+		}
+		struct key_def *pk_def = pk->def->key_def;
+
+		for (struct Index *idx = p->pIndex; idx != NULL;
+		     idx = idx->pNext) {
+			if (idx->index_type != SQL_INDEX_TYPE_CONSTRAINT_PK) {
+				struct index_def *def = idx->def;
+				struct key_def *cmp_def =
+					key_def_merge(def->key_def,
+						      pk_def);
+				if (cmp_def == NULL) {
+					pParse->rc = SQL_TARANTOOL_ERROR;
+					++pParse->nErr;
+					return;
+				}
+				if (!def->opts.is_unique) {
+					def->cmp_def->unique_part_count =
+						def->cmp_def->part_count;
+				} else {
+					def->cmp_def->unique_part_count =
+						def->key_def->part_count;
+				}
+			}
+		}
+	}

I don’t understand: why do you need at all this cmp_def?

In SQL it is extremely unlikely to be useful.

SQL has memtx/vinyl under it, where cmp_def is used, isn't it?
As far as I understand, there is some mechanism, which creates
struct index with struct index_def inside memtx (for example, for
memtx_tree_create()) based on what we passed inside in
createIndex() in sql/build.c
Is it right? I am confused.
If it is right, then we can simplify code in build by always
using key_def for cmp_def ?

 	if (db->init.busy) {
 		/*
 		 * As rebuild creates a new ExpList tree and
@@ -2398,8 +2441,7 @@
 struct Index *
 sql_index_alloc(struct sqlite3 *db)
 {
-	int index_size = ROUND8(sizeof(struct Index));
-	struct Index *p = sqlite3DbMallocZero(db, index_size);
+	struct Index *p = sqlite3DbMallocZero(db, sizeof(struct Index));
 	return p;
 }
 
@@ -2566,12 +2608,25 @@
 	if (parse->nErr > 0)
 		goto cleanup;
 
-	struct key_def *pk_key_def;
-	if (idx_type == SQL_INDEX_TYPE_UNIQUE ||
-	    idx_type == SQL_INDEX_TYPE_NON_UNIQUE)
+	struct key_def *pk_key_def = NULL;
+	if (idx_type != SQL_INDEX_TYPE_CONSTRAINT_PK &&
+	    parse->pNewTable == NULL)
 		pk_key_def = table->pIndex->def->key_def;
-	else
-		pk_key_def = NULL;
+	/*
+	 * In cases like CREATE TABLE t1(a UNIQUE, PRIMARY KEY(a))
+	 * fill_index_def() will be firstly executed for creating
+	 * index_def for UNIQUE constraint and then for creating
+	 * index_def for PRIMARY KEY constraint. So when
+	 * fill_index_def will be called for UNIQUE constraint
+	 * there will be no PK, while PK->def->key_def is needed
+	 * to create cmp_def (inside index_def_new()).

Again: explain pls why do you need cmp_def on client-side (i.e. parser)?

To handle
+	 * this case we set here pk_key_def to key_def and later
+	 * in sqlite3EndTable (when PK index is already created)
+	 * we go over all indexes and set appropriate cmp_def in
+	 * them.
+	 */
+	if (parse->pNewTable != NULL)
+		pk_key_def = key_def;
 
 	index->def = index_def_new(space_def->id, iid, name, name_len, TREE,
 				   &opts, key_def, pk_key_def);
@@ -2618,12 +2673,6 @@
 	 */
 	struct Table *table = NULL;
 	if (tbl_name != NULL) {
-		/*
-		 * Use the two-part index name to determine the
-		 * database to search for the table. 'Fix' the
-		 * table name to this db before looking up the
-		 * table.
-		 */
 		assert(token != NULL && token->z != NULL);
 		table = sqlite3LocateTable(parse, 0, tbl_name->a[0].zName);
 		assert(db->mallocFailed == 0 || table == NULL);
@@ -2642,7 +2691,7 @@
 	}
 	/*
 	 * Find the name of the index.  Make sure there is not
-	 * already another index or table with the same name.
+	 * already another index with the same name.
 	 *
 	 * Exception:  If we are reading the names of permanent
 	 * indices from the Tarantool schema (because some other
@@ -2660,7 +2709,7 @@
 	 * 2) UNIQUE constraint is non-named and standard
 	 *    auto-index name will be generated.
 	 */
-	bool is_constraint_named = false;
+	bool is_constraint_named;
 	if (token != NULL) {
 		name = sqlite3NameFromToken(db, token);
 		if (name == NULL)
@@ -2691,14 +2740,26 @@
 			}
 			name = sqlite3MPrintf(db, "sql_autoindex_%s_%d",
 					      table->def->name, n);
+			is_constraint_named = false;
 		} else {
-			name = sqlite3MPrintf(db, "sql_autoindex_%s",
-					      constraint_name);
+			/*
+			 * This naming is temporary. Now it's not
+			 * possible (since we implement UNIQUE
+			 * and PK constraints with indexes and
+			 * indexes can not have same names), but
+			 * in future we would use names exactly
+			 * as they are set by user.
+			 */
+			if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE)
+				name = sqlite3MPrintf(db,
+						      "unique_constraint_%s",
+						      constraint_name);
+			if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK)
+				name = sqlite3MPrintf(db, "pk_constraint_%s",
+						      constraint_name);
 			is_constraint_named = true;
 		}
-		if (constraint_name != NULL) {
-			sqlite3DbFree(db, constraint_name);
-		}
+		sqlite3DbFree(db, constraint_name);
 	}
 
 	if (name == NULL)
@@ -2708,7 +2769,8 @@
 			       table->def->id < BOX_SYSTEM_ID_MAX;
 	if (is_system_space && (idx_type == SQL_INDEX_TYPE_NON_UNIQUE ||
 				idx_type == SQL_INDEX_TYPE_UNIQUE)) {
-		diag_set(ClientError, ER_MODIFY_INDEX, name, table->def->name,
+		diag_set(ClientError, ER_MODIFY_INDEX, name,
+			 table->def->name,
 			 "can't create index on system space");
 		parse->nErr++;
 		parse->rc = SQL_TARANTOOL_ERROR;
@@ -2745,7 +2807,7 @@
 
 	index->pTable = table;
 	index->onError = (u8) on_error;
- 	index->index_type = idx_type;
+	index->index_type = idx_type;
 	index->pSchema = db->pSchema;
 	/* Tarantool have access to each column by any index. */
 	if (where != NULL) {
@@ -2775,8 +2837,22 @@
 			goto exit_create_index;
 	}
 
-	/* If it is parsing stage, then iid may have any value. */
-	uint32_t iid = 1;
+	/*
+	 * We set no SQL statement for indexes created during
+	 * CREATE TABLE statement. Instead we use
+	 * Index->index_def->opts.sql to store information if
+	 * this index implements named or non-named constraint.
+	 */
+	if (tbl_name == NULL &&
+	    idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE)

If idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE than tbl_name must be NULL:

you can’t declare unique constraint outside CREATE TABLE statement.

What about ALTER TABLE statements? We don't support ALTER TABLE ADD UNIQUE
now, but in future we want to have it, don't we?

+		sql_stmt = is_constraint_named ?
+			   "named constraint" : "non-named constraint”;
You can set ‘sql_stmt’ only for name constraints:

sql_stmt = is_constraint_name ? “_named_constr” : “”;

And use strncmp when comparing with it.

Also, if <if-clause> contains mode than one row,

we wrap it in brackets.

Fixed this.

+
+	/*
+	 * If it is parsing stage, then iid may have any value,
+	 * but PK still must have iid == 0
+	 */
+	uint32_t iid = idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK ? 0 : 1;
 	if (db->init.busy)
 		iid = SQLITE_PAGENO_TO_INDEXID(db->init.newTnum);

You can just do this:

uint32_t iid = idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK;

Ok. But =! instead of ==

+
+			if (k != key_def->part_count)
+				continue;
+
+			if (index->onError != existing_idx->onError) {
+				if (index->onError !=
+				    ON_CONFLICT_ACTION_DEFAULT &&
+				    existing_idx->onError !=
+				    ON_CONFLICT_ACTION_DEFAULT)
+					sqlite3ErrorMsg(parse,
+							"conflicting "\
+							"ON CONFLICT "\
+							"clauses specified”);

Again: here you have already set error. What is the reason to

continue process smth? You are able to go to exit_create_index.

Sorry, didn't understand you first time. Added goto.