Thank you for the review!
On 9 Jul 2018, at 13:46, Ivan Koptelov <ivan.koptelov@tarantool.org> wrote:
Sorry, in both previous letters patch was not properly formatted. Now I checked it
sending it to myself, it should be ok.
If you want to comment smth, you should do it after commit message,
near links to branch and issue - then, it would be omitted by git am.
Ok, will do so.
Also, again - you sent new version of patch, but didn’t attach changelong.
Sorry, I mistakenly squashed my bracnh and couldn't restore diff
from reflog.
Send with diff this time
--
sql: add index_def to Index
Now every sqlite struct Index is created with tnt struct
index_def inside. This allows us to use tnt index_def
in work with sqlite indexes in the same manner as with
tnt index and is a step to remove sqlite Index with
tnt index.
Fields coll_array, coll_id_array, aiColumn, sort_order,
aiRowLogEst and zName are removed from Index. All usages
of this fields changed to usage of corresponding index_def
or index_def->opts fields.
index_is_unique(), sql_index_collation() and
index_column_count() are removed with calls of
index_def corresponding fields.
Also there is small change in user-visible behavior:
On the contrary, this behaviour is not visible for user, it is like
things work under the hood.
Changed commit message.
before the patch a statement like
CREATE TABLE t1(a,b, PRIMARY KEY(a,b), UNIQUE(a,b))
created only one constraint index (for primary key)
and no index for UNIQUE constraint (since it is upon the
same columns), neither it is named or non-named constraint.
After the patch index will be always created for named
constraints. It is a temporary solution. In future it's
preferable not to create an index, but to make some record
in _constraints space that this named unique constraint
implemented with the same index as primary key constraint.
Not only PK constraints - any unique constraint.
I have found that named PK over named UNIQUE is not created:
create table t11(id int, constraint u1 unique(id), constraint pk1 primary key (id))
tarantool> select * from “_index"
…
- [350, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'unsigned']]]
- [512, 0, 'sql_autoindex_U1', 'tree', {'unique': true, 'sql': ''}, [{'sort_order': 'asc',
'type': 'integer', 'is_nullable': false, 'nullable_action': 'abort', 'field': 0}]]
PK and UNIQUE - different constraints (I don’t even say that it is named,
so must be created without any doubts).
This bug is fixed in the new version of the patch.
@@ -2511,44 +2431,11 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage)
sqlite3VdbeAddOp1(v, OP_Close, iSorter);
}
-/*
- * Allocate heap space to hold an Index object with nCol columns.
- *
- * Increase the allocation size to provide an extra nExtra bytes
- * of 8-byte aligned space after the Index object and return a
- * pointer to this extra space in *ppExtra.
- */
-Index *
-sqlite3AllocateIndexObject(sqlite3 * db, /* Database connection */
- i16 nCol, /* Total number of columns in the index */
- int nExtra, /* Number of bytes of extra space to alloc */
- char **ppExtra /* Pointer to the "extra" space */
- )
+struct Index *
+sql_index_alloc(struct sqlite3 *db)
{
- 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;
- }
+ int index_size = ROUND8(sizeof(struct Index));
+ struct Index *p = sqlite3DbMallocZero(db, index_size);
I said you twice to remove this strange alignment:
struct Index *p = sqlite3DbMallocZero(db, sizeof(*p));
Sorry, didn't get it before. Removed.
return p;
}
@@ -2635,48 +2522,132 @@ addIndexToTable(Index * pIndex, Table * pTab)
+static int
+index_fill_def(struct Parse *parse, struct Index *index,
+ struct Table *table, uint32_t iid, const char *name,
+ uint32_t name_len, struct ExprList *expr_list,
+ enum sql_index_type idx_type, char *sql_stmt)
+{
- return tnt_index->def->opts.is_unique;
+ struct key_def *pk_key_def;
+ if (idx_type == SQL_INDEX_TYPE_UNIQUE ||
+ idx_type == SQL_INDEX_TYPE_NON_UNIQUE)
Why not != SQL_INDEX_TYPE_CONSTRAINT_PK?
I rewrote this part. Now there is a trick. When we execute
statements like:
CREATE TABLE t1(id, CONSTRAINT u1 UNIQUE(id), CONSTRAINT pk1 PRIMARY
KEY(id));
we firstly create index and index_def for u1 constraint, then for
pk1.
But to create index_def for u1 we need to have pk index key_def to
create cmp_def. So if it is create table statement, then we use
key_def as pk_key_def to
create fake cmp_def in fill_index_def() and later, in
sqlite3EndTable (when we alreade created all indexes)
we set correct cmp_def using true pk_key_def.
+ pk_key_def = table->pIndex->def->key_def;
+ else
+ pk_key_def = NULL;
+
+ index->def = index_def_new(space_def->id, iid, name, name_len, TREE,
+ &opts, key_def, pk_key_def);
+ if (index->def == NULL)
+ goto tnt_error;
+ rc = 0;
+cleanup:
+ if (key_def != NULL)
+ key_def_delete(key_def);
+ return rc;
+tnt_error:
+ parse->rc = SQL_TARANTOOL_ERROR;
+ ++parse->nErr;
+ goto cleanup;
}
void
sql_create_index(struct Parse *parse, struct Token *token,
struct SrcList *tbl_name, struct ExprList *col_list,
- int on_error, struct Token *start, struct Expr *where,
- enum sort_order sort_order, bool if_not_exist, u8 idx_type)
-{
- Table *pTab = 0; /* Table to be indexed */
- Index *pIndex = 0; /* The index to be created */
- char *zName = 0; /* Name of the index */
- int nName; /* Number of characters in zName */
- int i, j;
- DbFixer sFix; /* For assigning database names to pTable */
- sqlite3 *db = parse->db;
- struct ExprList_item *col_listItem; /* For looping over col_list */
- int nExtra = 0; /* Space allocated for zExtra[] */
- char *zExtra = 0; /* Extra space after the Index object */
+ enum on_conflict_action on_error, struct Token *start,
+ struct Expr *where, enum sort_order sort_order,
+ bool if_not_exist, enum sql_index_type idx_type) {
+ /* The index to be created. */
+ struct Index *index = NULL;
+ /* Name of the index. */
+ char *name = NULL;
+ struct sqlite3 *db = parse->db;
struct session *user_session = current_session();
- if (db->mallocFailed || parse->nErr > 0) {
+ if (db->mallocFailed || parse->nErr > 0)
goto exit_create_index;
- }
- /* Do not account nested operations: the count of such
- * operations depends on Tarantool data dictionary internals,
- * such as data layout in system spaces. Also do not account
- * PRIMARY KEY and UNIQUE constraint - they had been accounted
- * in CREATE TABLE already.
+ /*
+ * Do not account nested operations: the count of such
+ * operations depends on Tarantool data dictionary
+ * internals, such as data layout in system spaces. Also
+ * do not account PRIMARY KEY and UNIQUE constraint - they
+ * had been accounted in CREATE TABLE already.
*/
- if (!parse->nested && idx_type == SQLITE_IDXTYPE_APPDEF) {
+ if (!parse->nested && (idx_type == SQL_INDEX_TYPE_UNIQUE ||
+ idx_type == SQL_INDEX_TYPE_NON_UNIQUE)) {
Vdbe *v = sqlite3GetVdbe(parse);
if (v == NULL)
goto exit_create_index;
@@ -2685,39 +2656,30 @@ sql_create_index(struct Parse *parse, struct Token *token,
assert(db->pSchema != NULL);
/*
- * Find the table that is to be indexed. Return early if not found.
+ * Find the table that is to be indexed.
+ * Return early if not found.
*/
+ 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.
+ /*
+ * 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.
*/
You have already deleted ‘fixing’ routine. Remove this comment.
Removed.
- assert(token && token->z);
-
- sqlite3FixInit(&sFix, parse, "index", token);
- if (sqlite3FixSrcList(&sFix, tbl_name)) {
- /* Because the parser constructs tbl_name from a single identifier,
- * sqlite3FixSrcList can never fail.
- */
- assert(0);
- }
- pTab = sqlite3LocateTable(parse, 0, tbl_name->a[0].zName);
- assert(db->mallocFailed == 0 || pTab == 0);
- if (pTab == 0)
- goto exit_create_index;
- sqlite3PrimaryKeyIndex(pTab);
+ assert(token != NULL && token->z != NULL);
+ table = sqlite3LocateTable(parse, 0, tbl_name->a[0].zName);
+ assert(db->mallocFailed == 0 || table == NULL);
} else {
assert(token == NULL);
assert(start == NULL);
- pTab = parse->pNewTable;
- if (!pTab)
- goto exit_create_index;
+ table = parse->pNewTable;
}
- assert(pTab != 0);
- assert(parse->nErr == 0);
- if (pTab->def->opts.is_view) {
+ if (table == NULL || parse->nErr > 0)
+ goto exit_create_index;
+
+ if (table->def->opts.is_view) {
sqlite3ErrorMsg(parse, "views may not be indexed");
goto exit_create_index;
}
@@ -2734,45 +2696,68 @@ sql_create_index(struct Parse *parse, struct Token *token,
* If token == NULL it means that we are dealing with a
* primary key or UNIQUE constraint. We have to invent
* our own name.
Fix also and this comment: it starts from:
* Find the name of the index. Make sure there is not
* already another index or table with the same name.
*
As we discussed, we allow index to be named as table.
Sorry, fixed.
+ *
+ * In case of UNIQUE constraint we have two options:
+ * 1) UNIQUE constraint is named and this name will
+ * be a part of index name.
+ * 2) UNIQUE constraint is non-named and standard
+ * auto-index name will be generated.
*/
- if (token) {
- zName = sqlite3NameFromToken(db, token);
- if (zName == 0)
+ bool is_constraint_named = false;
+ if (token != NULL) {
+ name = sqlite3NameFromToken(db, token);
+ if (name == NULL)
goto exit_create_index;
- assert(token->z != 0);
- if (!db->init.busy) {
- if (sqlite3HashFind(&db->pSchema->tblHash, zName) !=
- NULL) {
- sqlite3ErrorMsg(parse,
- "there is already a table named %s",
- zName);
- goto exit_create_index;
- }
- }
- if (sqlite3HashFind(&pTab->idxHash, zName) != NULL) {
+ assert(token->z != NULL);
+ if (sqlite3HashFind(&table->idxHash, name) != NULL) {
if (!if_not_exist) {
sqlite3ErrorMsg(parse,
"index %s.%s already exists",
- pTab->def->name, zName);
+ table->def->name, name);
} else {
assert(!db->init.busy);
}
goto exit_create_index;
}
} else {
- int n;
- Index *pLoop;
- for (pLoop = pTab->pIndex, n = 1; pLoop;
- pLoop = pLoop->pNext, n++) {
+ char *constraint_name = NULL;
+ if (parse->constraintName.z != NULL)
+ constraint_name =
+ sqlite3NameFromToken(db, &parse->constraintName);
Out of 80.
I see that it is 75. Anyway, splited the line.
+
+ if (constraint_name == NULL ||
+ strcmp(constraint_name, "") == 0) {
+ int n = 1;
+ for (struct Index *idx = table->pIndex;
+ idx != NULL;
+ idx = idx->pNext, n++) {
+ }
+ name = sqlite3MPrintf(db, "sql_autoindex_%s_%d",
+ table->def->name, n);
+ } else {
+ name = sqlite3MPrintf(db, "sql_autoindex_%s",
+ constraint_name);
+ is_constraint_named = true;
Lets use another prefix than ‘sql_autoindex_’ for named constraints,
like ‘unique_constraint_’. And leave comment describing this naming is temporary.
Fixed to `unique_constraint_`.
}
- zName =
- sqlite3MPrintf(db, "sqlite_autoindex_%s_%d", pTab->def->name,
- n);
- if (zName == 0) {
- goto exit_create_index;
+ if (constraint_name != NULL) {
+ sqlite3DbFree(db, constraint_name);
You don’t need this check: sqlite3DbFree tests on NULL ptr itself.
+ if (table == parse->pNewTable) {
+ for (struct Index *idx = table->pIndex; idx != NULL;
+ idx = idx->pNext) {
+ uint32_t k;
+ assert(IsUniqueIndex(idx));
+ assert(IsUniqueIndex(index));
+
+ if (idx->def->key_def->part_count !=
+ index->def->key_def->part_count)
continue;
- for (k = 0; k < pIdx->nColumn; k++) {
- assert(pIdx->aiColumn[k] >= 0);
- if (pIdx->aiColumn[k] != pIndex->aiColumn[k])
+ for (k = 0; k < idx->def->key_def->part_count; k++) {
+ if (idx->def->key_def->parts[k].fieldno !=
+ index->def->key_def->parts[k].fieldno)
break;
struct coll *coll1, *coll2;
- uint32_t id;
- coll1 = sql_index_collation(pIdx, k, &id);
- coll2 = sql_index_collation(pIndex, k, &id);
+ coll1 = idx->def->key_def->parts[k].coll;
+ coll2 = index->def->key_def->parts[k].coll;
if (coll1 != coll2)
break;
}
- if (k == pIdx->nColumn) {
- if (pIdx->onError != pIndex->onError) {
- /* This constraint creates the same index as a previous
- * constraint specified somewhere in the CREATE TABLE statement.
- * However the ON CONFLICT clauses are different. If both this
- * constraint and the previous equivalent constraint have explicit
- * ON CONFLICT clauses this is an error. Otherwise, use the
- * explicitly specified behavior for the index.
+ if (k == idx->def->key_def->part_count) {
+ if (idx->onError != index->onError) {
+ /*
+ * This constraint creates
+ * the same index as a
+ * previous
+ * constraint specified
+ * somewhere in the CREATE
+ * TABLE statement.
+ * However the ON CONFLICT
+ * clauses are different.
+ * If both this constraint
+ * and the previous
+ * equivalent constraint
+ * have explicit
+ * ON CONFLICT clauses
+ * this is an error.
+ * Otherwise, use the
+ * explicitly specified
+ * behavior for the index.
*/
Cmon, it looks so ugly. Lets move this comment somewhere.
Btw, AFAIU 66 is only recommendation, so in case of emergency you can break this rule.
Rewroted this part.
- if (!
- (pIdx->onError == ON_CONFLICT_ACTION_DEFAULT
- || pIndex->onError ==
- ON_CONFLICT_ACTION_DEFAULT)) {
+ if (idx->onError !=
+ ON_CONFLICT_ACTION_DEFAULT &&
+ index->onError !=
+ ON_CONFLICT_ACTION_DEFAULT) {
sqlite3ErrorMsg(parse,
- "conflicting ON CONFLICT clauses specified",
- 0);
- }
- if (pIdx->onError == ON_CONFLICT_ACTION_DEFAULT) {
- pIdx->onError = pIndex->onError;
+ "conflicting "\
+ "ON CONFLICT "\
+ "clauses "\
+ "specified”);
Why do you continue processing here?
Because if there is no conflicting clauses then we want to prevent
creating index for
non-named UNIQUE constraints. I rewroted this part, now it is
clearer.
}
+ if (idx->onError ==
+ ON_CONFLICT_ACTION_DEFAULT)
+ idx->onError = index->onError;
If you exit on error above, condition under this if statement
will be always evaluated to true.
Didn't understand you. The idea of this is to handle this:
we have two constraints on the same columns. One has default
conflict behavior,
another has specified. To avoid ambiguity we set specified type of
behavior in both
constraints.
+ }
+ if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK)
+ idx->index_type = idx_type;
Why do you need this if? You always assign idx->index_type = idx_type.
Rewroted this. Originaly this line handled the following case:
CREATE TABLE t(a, UNIQUE(a), PRIMARY KEY(a))
This line changed type of UNIQUE constraint index to be PK index.
Index for PK constraint was not created.
I described in comments how we do it now.
@@ -2880,9 +2865,7 @@ whereLoopAddBtree(WhereLoopBuilder * pBuilder, /* WHERE clause information */
{
WhereInfo *pWInfo; /* WHERE analysis context */
Index *pProbe; /* An index we are evaluating */
- Index sPk; /* A fake index object for the primary key */
- LogEst aiRowEstPk[2]; /* The aiRowLogEst[] value for the sPk index */
- i16 aiColumnPk = -1; /* The aColumn[] value for the sPk index */
+ Index fake_index; /* A fake index object for the primary key */
SrcList *pTabList; /* The FROM clause */
struct SrcList_item *pSrc; /* The FROM clause btree term to add */
WhereLoop *pNew; /* Template WhereLoop object */
@@ -2903,31 +2886,62 @@ whereLoopAddBtree(WhereLoopBuilder * pBuilder, /* WHERE clause information */
if (pSrc->pIBIndex) {
/* An INDEXED BY clause specifies a particular index to use */
pProbe = pSrc->pIBIndex;
+ fake_index.def = NULL;
} else if (pTab->pIndex) {
pProbe = pTab->pIndex;
+ fake_index.def = NULL;
} else {
/* There is no INDEXED BY clause. Create a fake Index object in local
- * variable sPk to represent the primary key index. Make this
+ * variable fake_index to represent the primary key index. Make this
* fake index the first in a chain of Index objects with all of the real
* indices to follow
*/
Index *pFirst; /* First of real indices on the table */
- memset(&sPk, 0, sizeof(Index));
- sPk.nColumn = 1;
- sPk.aiColumn = &aiColumnPk;
- sPk.aiRowLogEst = aiRowEstPk;
- sPk.onError = ON_CONFLICT_ACTION_REPLACE;
- sPk.pTable = pTab;
- aiRowEstPk[0] = sql_space_tuple_log_count(pTab);
- aiRowEstPk[1] = 0;
+ memset(&fake_index, 0, sizeof(Index));
+ fake_index.onError = ON_CONFLICT_ACTION_REPLACE;
+ fake_index.pTable = pTab;
+
+ struct key_def *key_def = key_def_new(1);
+ if (key_def == NULL) {
+ pWInfo->pParse->nErr++;
+ pWInfo->pParse->rc = SQL_TARANTOOL_ERROR;
+ return SQL_TARANTOOL_ERROR;
+ }
+
+ key_def_set_part(key_def, 0, 0, pTab->def->fields[0].type,
+ ON_CONFLICT_ACTION_ABORT,
+ NULL, COLL_NONE, SORT_ORDER_ASC);
+
+ struct index_opts opts;
+ index_opts_create(&opts);
+ opts.sql = "fake_autoindex";
+ fake_index.def = index_def_new(pTab->def->id, 0, "fake_autoindex”,
Out of 80.
Fixed.
Here is the diff:
===================================================================
--- src/box/sql/build.c (date 1528443121000)
+++ src/box/sql/build.c (date 1531497008000)
@@ -1360,8 +1360,8 @@
SQL_SUBTYPE_MSGPACK,zParts, P4_STATIC);
sqlite3VdbeAddOp3(v, OP_MakeRecord, iFirstCol, 6, iRecord);
sqlite3VdbeAddOp2(v, OP_SInsert, BOX_INDEX_ID, iRecord);
- if (pIndex->idxType == SQL_INDEX_TYPE_NON_UNIQUE ||
- pIndex->idxType == SQL_INDEX_TYPE_UNIQUE)
+ if (pIndex->index_type == SQL_INDEX_TYPE_NON_UNIQUE ||
+ pIndex->index_type == SQL_INDEX_TYPE_UNIQUE)
sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
}
@@ -1665,6 +1665,49 @@
return;
}
+ /*
+ * 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;
+ }
+ }
+ }
+ }
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()). 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)
+ sql_stmt = is_constraint_named ?
+ "named constraint" : "non-named constraint";
+
+ /*
+ * 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);
@@ -2807,73 +2883,96 @@
if (!index_def_is_valid(index->def, table->def->name))
goto exit_create_index;
+ /*
+ * Here we handle cases, when in CREATE TABLE statement
+ * some UNIQUE constraints are putted exactly on the same
+ * columns with PRIMARY KEY constraint. Our general
+ * intention is to omit creating indexes for non-named
+ * UNIQUE constraints if these constraints are putted on
+ * the same columns as the PRIMARY KEY constraint. In
+ * different cases it is implemented in different ways.
+ *
+ * 1) CREATE TABLE t(a UNIQUE PRIMARY KEY)
+ * CREATE TABLE t(a, UNIQUE(a), PRIMARY KEY(a))
+ * In these cases we firstly proceed UNIQUE(a) expression
+ * and create index for it, then proceed PRIMARY KEY,
+ * but don't create index for it. Instead of it we
+ * change UNIQUE constraint index name and index_type,
+ * so it becomes PRIMARY KEY index.
+ *
+ * 2) CREATE TABLE t(a, PRIMARY KEY(a), UNIQUE(a))
+ * In such cases we simply do not create index for
+ * UNIQUE constraint.
+ *
+ * Note 1: We always create new index for named UNIQUE
+ * constraints.
+ *
+ * Note 2: If UNIQUE constraint (no matter named or non-named)
+ * is putted on the same columns as PRIMARY KEY constraint,
+ * but has different onError (behavior on constraint violation),
+ * then an error is raised.
+ */
if (table == parse->pNewTable) {
- for (struct Index *idx = table->pIndex; idx != NULL;
- idx = idx->pNext) {
+ for (struct Index *existing_idx = table->pIndex;
+ existing_idx != NULL;
+ existing_idx = existing_idx->pNext) {
+
+ struct key_def *key_def = index->def->key_def;
+ struct key_def *exst_key_def =
+ existing_idx->def->key_def;
+
+ if (key_def->part_count != exst_key_def->part_count)
+ continue;
+
uint32_t k;
- assert(IsUniqueIndex(idx));
- assert(IsUniqueIndex(index));
-
- if (idx->def->key_def->part_count !=
- index->def->key_def->part_count)
- continue;
- for (k = 0; k < idx->def->key_def->part_count; k++) {
- if (idx->def->key_def->parts[k].fieldno !=
- index->def->key_def->parts[k].fieldno)
+ for (k = 0; k < key_def->part_count; k++) {
+ if (key_def->parts[k].fieldno !=
+ exst_key_def->parts[k].fieldno)
break;
- struct coll *coll1, *coll2;
- coll1 = idx->def->key_def->parts[k].coll;
- coll2 = index->def->key_def->parts[k].coll;
- if (coll1 != coll2)
+ if (key_def->parts[k].coll !=
+ exst_key_def->parts[k].coll)
break;
}
- if (k == idx->def->key_def->part_count) {
- if (idx->onError != index->onError) {
- /*
- * This constraint creates
- * the same index as a
- * previous
- * constraint specified
- * somewhere in the CREATE
- * TABLE statement.
- * However the ON CONFLICT
- * clauses are different.
- * If both this constraint
- * and the previous
- * equivalent constraint
- * have explicit
- * ON CONFLICT clauses
- * this is an error.
- * Otherwise, use the
- * explicitly specified
- * behavior for the index.
- */
- if (idx->onError !=
- ON_CONFLICT_ACTION_DEFAULT &&
- index->onError !=
- ON_CONFLICT_ACTION_DEFAULT) {
- sqlite3ErrorMsg(parse,
- "conflicting "\
- "ON CONFLICT "\
- "clauses "\
- "specified");
- }
- if (idx->onError ==
- ON_CONFLICT_ACTION_DEFAULT)
- idx->onError = index->onError;
- }
- if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK)
- idx->index_type = idx_type;
+
+ 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");
+
+ if (existing_idx->onError ==
+ ON_CONFLICT_ACTION_DEFAULT)
+ existing_idx->onError = index->onError;
+ }
+
+ bool is_exst_named =
+ strcmp(existing_idx->def->opts.sql,
+ "named constraint") == 0;
- if(is_constraint_named &&
- idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE) {
- continue;
+ /* CREATE TABLE t(a, UNIQUE(a), PRIMARY KEY(a)). */
+ if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK) {
+ if (existing_idx->index_type ==
+ SQL_INDEX_TYPE_CONSTRAINT_UNIQUE &&
+ !is_exst_named) {
+ existing_idx->index_type =
+ SQL_INDEX_TYPE_CONSTRAINT_PK;
+ goto exit_create_index;
}
+ }
+
+ /* CREATE TABLE t(a, PRIMARY KEY(a), UNIQUE(a)). */
+ if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_UNIQUE &&
+ !is_constraint_named)
goto exit_create_index;
- }
}
}
-
/*
* Link the new Index structure to its table and to the
* other in-memory database structures.
--- src/box/sql/where.c (date 1528443121000)
+++ src/box/sql/where.c (date 1531497008000)
@@ -2915,9 +2915,10 @@
struct index_opts opts;
index_opts_create(&opts);
opts.sql = "fake_autoindex";
- fake_index.def = index_def_new(pTab->def->id, 0, "fake_autoindex",
- sizeof("fake_autoindex") - 1, TREE,
- &opts, key_def, NULL);
+ fake_index.def =
+ index_def_new(pTab->def->id, 0,"fake_autoindex",
+ sizeof("fake_autoindex") - 1,
+ TREE, &opts, key_def, NULL);
key_def_delete(key_def);
if (fake_index.def == NULL) {
--- test/sql-tap/index7.test.lua (date 1528443121000)
+++ test/sql-tap/index7.test.lua (date 1531497008000)
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(9)
+test:plan(11)
--!./tcltestrunner.lua
-- 2013-11-04
@@ -350,12 +350,12 @@
"index7-8.3",
[[
CREATE TABLE test5(a,b,c,d, PRIMARY KEY(a), UNIQUE(a));
- SELECT "_index"."name"
+ SELECT "_index"."name", "_index"."iid"
FROM "_index" JOIN "_space" WHERE
"_index"."id" = "_space"."id" AND
"_space"."name"='TEST5';
]],
- {0, {'sql_autoindex_TEST5_1'}})
+ {0, {"sql_autoindex_TEST5_1",0}})
-- This test checks that CREATE TABLE statement with PK constraint
-- and NAMED UNIQUE constraint (declared on THE SAME COLUMNS)
@@ -366,11 +366,41 @@
"index7-8.4",
[[
CREATE TABLE test6(a,b,c,d, PRIMARY KEY(a), CONSTRAINT c1 UNIQUE(a));
- SELECT "_index"."name"
+ SELECT "_index"."name", "_index"."iid"
FROM "_index" JOIN "_space" WHERE
"_index"."id" = "_space"."id" AND
"_space"."name"='TEST6';
]],
- {0, {"sql_autoindex_TEST6_1", "sql_autoindex_C1"}})
+ {0, {"sql_autoindex_TEST6_1",0,"unique_constraint_C1",1}})
+
+-- This test checks that CREATE TABLE statement with PK constraint
+-- and UNIQUE constraint is executed correctly
+-- (no matter if UNIQUE precedes PK or not).
+--
+test:do_catchsql_test(
+ "index7-8.5",
+ [[
+ CREATE TABLE test7(a,b,c,d, UNIQUE(a), PRIMARY KEY(a));
+ SELECT "_index"."name", "_index"."iid"
+ FROM "_index" JOIN "_space" WHERE
+ "_index"."id" = "_space"."id" AND
+ "_space"."name"='TEST7';
+ ]],
+ {0, {"sql_autoindex_TEST7_1",0}})
+
+
+-- This test is the same as previous, but with named UNIQUE
+-- constraint.
+--
+test:do_catchsql_test(
+ "index7-8.6",
+ [[
+ CREATE TABLE test8(a,b,c,d, CONSTRAINT c1 UNIQUE(a), PRIMARY KEY(a));
+ SELECT "_index"."name", "_index"."iid"
+ FROM "_index" JOIN "_space" WHERE
+ "_index"."id" = "_space"."id" AND
+ "_space"."name"='TEST8';
+ ]],
+ {0, {"sql_autoindex_TEST8_2",0,"unique_constraint_C1",1}})
test:finish_test()