[tarantool-patches] Re: [PATCH 2/7] sql: remove SQLite original struct Index
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Thu Sep 6 22:54:27 MSK 2018
Hi! Thanks for the fixes! See my own ones
on the branch and comments below.
>>> + assert(space != NULL);
>>> + for (uint32_t i = 0; i < space->index_count; ++i) {
>>> + struct index *idx = space->index[i];
>>
>> 2. You have mentioned that space indexes are stored as an
>> array, but it is not the only true. The indexes are stored in
>> two ways: as an array and a map. A map allows to find an index
>> by id in O(1) time. Look at space_index function.
>
> It is cool and I know about it, but the most common use case is simple
> iteration over all indexes like:
> for (i = 0; i < space->index_count; ++i) {
> struct index *idx = space->index[I];
> ...
> }>
>> Here you do not clear space->index_map and do not
>> reallocate it. Looks like you do not touch index_map
>> at all in this patch. But maybe you should. See my
>> other comments here and in next patches.
>
> I deliberately didn’t fill in index map. I think maintaining
> map and array at the same time seems to be over-engineering.
> These indexes are really used only during table creation
> (after last patch in series). Moreover, during building routine
> we may need only PK index, which anyway can be fetched as index[0]
It is not over-engineering. This is how real spaces work and I
want these surrogate spaces be as much similar as possible to
them. So I implemented index_map support in my review fixes.
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index d1d952330..617d0ea47 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -260,38 +252,39 @@ table_column_is_in_pk(Table *table, uint32_t column)
> return false;
> }
>
> -/*
> +/**
> * Remove the memory data structures associated with the given
> - * Table. No changes are made to disk by this routine.
> - *
> - * This routine just deletes the data structure. It does not unlink
> - * the table data structure from the hash table. But it does destroy
> - * memory structures of the indices and foreign keys associated with
> - * the table.
> + * Table.
> *
> - * The db parameter is optional. It is needed if the Table object
> - * contains lookaside memory. (Table objects in the schema do not use
> - * lookaside memory, but some ephemeral Table objects do.) Or the
> - * db parameter can be used with db->pnBytesFreed to measure the memory
> - * used by the Table object.
> + * @param db Database handler.
> + * @param tab Table to be deleted.
> */
> -static void SQLITE_NOINLINE
> -deleteTable(sqlite3 * db, Table * pTable)
> +static void
> +table_delete(struct sqlite3 *db, struct Table *tab)
> {
> - Index *pIndex, *pNext;
> -
> + if (tab->space->def != NULL)
> + goto skip_index_delete;
> /* Delete all indices associated with this table. */
> - for (pIndex = pTable->pIndex; pIndex; pIndex = pNext) {
> - pNext = pIndex->pNext;
> - freeIndex(db, pIndex);
> - }
> - assert(pTable->def != NULL);
> - /* Do not delete pTable->def allocated on region. */
> - if (!pTable->def->opts.is_temporary)
> - space_def_delete(pTable->def);
> + for (uint32_t i = 0; i < tab->space->index_count; ++i) {
> + /*
> + * These indexes are just wrapper for
> + * index_def's, so it makes no sense to call
> + * index_delete().
> + */
> + struct index *idx = tab->space->index[i];
> + free(idx->def);
Leak of idx->def->key_def/cmp_def.
> + free(idx);
> + }
> + free(tab->space->index);
> + free(tab->space);
> +skip_index_delete:
> + assert(tab->def != NULL);
> + /* Do not delete table->def allocated on region. */
> + if (!tab->def->opts.is_temporary)
> + space_def_delete(tab->def);
> else
> - sql_expr_list_delete(db, pTable->def->opts.checks);
> - sqlite3DbFree(db, pTable);
> + sql_expr_list_delete(db, tab->def->opts.checks);
> + sqlite3DbFree(db, tab);
> }
>
> void
> @@ -1156,137 +1137,132 @@ getNewSpaceId(Parse * pParse)
> return iRes;
> }
>
> -/*
> - * Generate VDBE code to create an Index. This is acomplished by adding
> - * an entry to the _index table. ISpaceId either contains the literal
> - * space id or designates a register storing the id.
> +/**
> + * Generate VDBE code to create an Index. This is accomplished by
> + * adding an entry to the _index table.
> + *
> + * @param parse Current parsing context.
> + * @param def Definition of space which index belongs to.
> + * @param idx_def Definition of index under construction.
> + * @param pk_def Definition of primary key index.
> + * @param space_id_reg Register containing generated space id.
> */
> static void
> -createIndex(Parse * pParse, Index * pIndex, int iSpaceId, int iIndexId,
> - const char *zSql)
> +vdbe_emit_create_index(struct Parse *parse, struct space_def *def,
> + const struct index_def *idx_def,
> + const struct index_def *pk_def, int space_id_reg)
> {
> - Vdbe *v = sqlite3GetVdbe(pParse);
> - int iFirstCol = ++pParse->nMem;
> - int iRecord = (pParse->nMem += 6); /* 6 total columns */
> - struct index_opts opts = pIndex->def->opts;
> - opts.sql = (char *)zSql;
> - struct region *region = &pParse->region;
> + struct Vdbe *v = sqlite3GetVdbe(parse);
> + int entry_reg = ++parse->nMem;
> + /*
> + * Entry in _index space contains 6 fields.
> + * The last one contains encoded tuple.
> + */
> + int tuple_reg = (parse->nMem += 6);
> + /* Format "opts" and "parts" for _index entry. */
> + struct region *region = &parse->region;
> uint32_t index_opts_sz = 0;
> - char *index_opts =
> - sql_encode_index_opts(region, &opts, &index_opts_sz);
> + char *index_opts = sql_encode_index_opts(region, &idx_def->opts,
> + &index_opts_sz);
> if (index_opts == NULL)
> goto error;
> uint32_t index_parts_sz = 0;
> - char *index_parts =
> - sql_encode_index_parts(region, pIndex, &index_parts_sz);
> + char *index_parts = sql_encode_index_parts(region, def->fields, idx_def,
> + pk_def, &index_parts_sz);
> if (index_parts == NULL)
> goto error;
> - char *raw = sqlite3DbMallocRaw(pParse->db,
> - index_opts_sz + index_parts_sz);
> + char *raw = sqlite3DbMallocRaw(parse->db,
> + index_opts_sz +index_parts_sz);
> if (raw == NULL)
> return;
> -
> memcpy(raw, index_opts, index_opts_sz);
> index_opts = raw;
> raw += index_opts_sz;
> memcpy(raw, index_parts, index_parts_sz);
> index_parts = raw;
>
> - if (pParse->pNewTable) {
> - int reg;
> - /*
> - * A new table is being created, hence iSpaceId is a register, but
> - * iIndexId is literal.
> - */
> - sqlite3VdbeAddOp2(v, OP_SCopy, iSpaceId, iFirstCol);
> - sqlite3VdbeAddOp2(v, OP_Integer, iIndexId, iFirstCol + 1);
> -
> - /* Generate code to save new pageno into a register.
> - * This is runtime implementation of SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID:
> - * pageno = (spaceid << 10) | indexid
> - */
> - pParse->regRoot = ++pParse->nMem;
> - reg = ++pParse->nMem;
> - sqlite3VdbeAddOp2(v, OP_Integer, 1 << 10, reg);
> - sqlite3VdbeAddOp3(v, OP_Multiply, reg, iSpaceId,
> - pParse->regRoot);
> - sqlite3VdbeAddOp3(v, OP_AddImm, pParse->regRoot, iIndexId,
> - pParse->regRoot);
> + if (parse->pNewTable != NULL) {
> + sqlite3VdbeAddOp2(v, OP_SCopy, space_id_reg, entry_reg);
> + sqlite3VdbeAddOp2(v, OP_Integer, idx_def->iid, entry_reg + 1);
> } else {
> /*
> - * An existing table is being modified; iSpaceId is literal, but
> - * iIndexId is a register.
> + * An existing table is being modified;
> + * space_id_reg is register, but iid is literal.
The comment is wrong. Here space_id_reg is literal
and idx_def->iid is a register. And here a problem is
hidden. idx_def->iid is initialized above as either 0
for primary indexes or 1 for other. But by a coincidence
it matched result of getNewIid() so it worked ok. Once
I added index_map and more strict index identifiers
calculation, this place fired. It is fixed in my
review fixes.
> diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
> index efc531bbd..f3e65303b 100644
> --- a/src/box/sql/tarantoolInt.h
> +++ b/src/box/sql/tarantoolInt.h
> @@ -207,6 +207,7 @@ fkey_encode_links(struct region *region, const struct fkey_def *def, int type,
> uint32_t *size);
>
> /**
> +<<<<<<< HEAD
Ooops.
> * Encode index parts of given foreign key constraint into
> * MsgPack on @region.
> * @param region Region to use.
Fixes:
================================================================
commit dbeb4281577df192f1cbbc670070de74c93dea39
Author: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
Date: Thu Sep 6 20:18:17 2018 +0300
Review fixes
diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
index e856d64c7..2c54c1835 100644
--- a/extra/mkkeywordhash.c
+++ b/extra/mkkeywordhash.c
@@ -82,11 +82,6 @@ struct Keyword {
#else
# define PRAGMA 0x00000400
#endif
-#ifdef SQLITE_OMIT_REINDEX
-# define REINDEX 0
-#else
-# define REINDEX 0x00000800
-#endif
#define SUBQUERY 0x00001000
# define TRIGGER 0x00002000
# define VIEW 0x00008000
diff --git a/src/box/sql.c b/src/box/sql.c
index 0c1df9b75..97948ab3b 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1297,7 +1297,7 @@ sql_encode_table(struct region *region, struct Table *table, uint32_t *size)
* If table's PK is single column which is INTEGER, then
* treat it as strict type, not affinity.
*/
- struct index *pk_idx = sql_table_primary_key(table);
+ struct index *pk_idx = space_index(table->space, 0);
uint32_t pk_forced_int = UINT32_MAX;
if (pk_idx != NULL && pk_idx->def->key_def->part_count == 1) {
int pk = pk_idx->def->key_def->parts[0].fieldno;
@@ -1659,21 +1659,30 @@ sql_ephemeral_table_new(Parse *parser, const char *name)
Table *table = sqlite3DbMallocZero(db, sizeof(Table));
if (table != NULL)
def = sql_ephemeral_space_def_new(parser, name);
- if (def == NULL) {
- sqlite3DbFree(db, table);
- return NULL;
- }
+ if (def == NULL)
+ goto err_def;
table->space = (struct space *) calloc(1, sizeof(struct space));
if (table->space == NULL) {
diag_set(OutOfMemory, sizeof(struct space), "calloc", "space");
- parser->rc = SQL_TARANTOOL_ERROR;
- parser->nErr++;
- sqlite3DbFree(db, table);
- return NULL;
+ goto err_space;
+ }
+ table->space->index_map =
+ (struct index **) calloc(1, sizeof(struct index *));
+ if (table->space->index_map == NULL) {
+ diag_set(OutOfMemory, sizeof(struct index *), "calloc",
+ "table->space->index_map");
+ goto err_map;
}
-
table->def = def;
return table;
+err_map:
+ free(table->space);
+err_space:
+ parser->rc = SQL_TARANTOOL_ERROR;
+ parser->nErr++;
+err_def:
+ sqlite3DbFree(db, table);
+ return NULL;
}
int
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 80293c6ed..1c9a672d4 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -802,7 +802,14 @@ analyzeOneTable(Parse * pParse, /* Parser context */
return;
}
assert(pTab->def->id != 0);
- if (sqlite3_strlike("\\_%", pTab->def->name, '\\') == 0) {
+ /*
+ * Here we need real space from Tarantool DD since
+ * further it is passed to cursor opening routine.
+ */
+ struct space *space = space_by_id(pTab->def->id);
+ assert(space != NULL);
+ const char *tab_name = space_name(space);
+ if (sqlite3_strlike("\\_%", tab_name, '\\') == 0) {
/* Do not gather statistics on system tables */
return;
}
@@ -816,15 +823,9 @@ analyzeOneTable(Parse * pParse, /* Parser context */
iIdxCur = iTab++;
pParse->nTab = MAX(pParse->nTab, iTab);
sqlite3OpenTable(pParse, iTabCur, pTab, OP_OpenRead);
- sqlite3VdbeLoadString(v, regTabname, pTab->def->name);
- /*
- * Here we need real space from Tarantool DD since
- * further it is passed to cursor opening routine.
- */
- struct space *space = space_by_id(pTab->def->id);
- assert(space != NULL);
+ sqlite3VdbeLoadString(v, regTabname, tab_name);
for (uint32_t j = 0; j < space->index_count; ++j) {
- struct index *idx = pTab->space->index[j];
+ struct index *idx = space->index[j];
int addrRewind; /* Address of "OP_Rewind iIdxCur" */
int addrNextRow; /* Address of "next_row:" */
const char *idx_name; /* Name of the index */
@@ -834,15 +835,14 @@ analyzeOneTable(Parse * pParse, /* Parser context */
* instead more familiar table name.
*/
if (idx->def->iid == 0)
- idx_name = pTab->def->name;
+ idx_name = tab_name;
else
idx_name = idx->def->name;
int part_count = idx->def->key_def->part_count;
/* Populate the register containing the index name. */
sqlite3VdbeLoadString(v, regIdxname, idx_name);
- VdbeComment((v, "Analysis for %s.%s", pTab->def->name,
- idx_name));
+ VdbeComment((v, "Analysis for %s.%s", tab_name, idx_name));
/*
* Pseudo-code for loop that calls stat_push():
@@ -986,7 +986,7 @@ analyzeOneTable(Parse * pParse, /* Parser context */
* if !eof(csr) goto next_row;
*/
assert(regKey == (regStat4 + 2));
- struct index *pk = sql_table_primary_key(pTab);
+ struct index *pk = space_index(space, 0);
int pk_part_count = pk->def->key_def->part_count;
/* Allocate memory for array. */
pParse->nMem = MAX(pParse->nMem,
@@ -994,10 +994,10 @@ analyzeOneTable(Parse * pParse, /* Parser context */
int regKeyStat = regPrev + part_count;
for (i = 0; i < pk_part_count; i++) {
uint32_t k = pk->def->key_def->parts[i].fieldno;
- assert(k < pTab->def->field_count);
+ assert(k < space->def->field_count);
sqlite3VdbeAddOp3(v, OP_Column, iIdxCur, k,
regKeyStat + i);
- VdbeComment((v, "%s", pTab->def->fields[k].name));
+ VdbeComment((v, "%s", space->def->fields[k].name));
}
sqlite3VdbeAddOp3(v, OP_MakeRecord, regKeyStat,
pk_part_count, regKey);
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 617d0ea47..d708dbeda 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -159,40 +159,52 @@ sqlite3LocateIndex(sqlite3 * db, const char *zName, const char *zTable)
return NULL;
}
-void
+int
sql_space_index_delete(struct space *space, uint32_t iid)
{
assert(space != NULL);
+ struct index *to_delete = NULL;
+ uint32_t new_index_id_max = 0;
for (uint32_t i = 0; i < space->index_count; ++i) {
- struct index *idx = space->index[i];
- /*
- * Allocate new chunk with size reduced by 1 slot.
- * Copy all indexes to that chunk except for one
- * to be deleted.
- */
- if (idx->def->iid == iid) {
- index_def_delete(idx->def);
- free(idx);
- size_t idx_sz = sizeof(struct index *);
- uint32_t idx_count = space->index_count - 1;
- struct index **new_indexes =
- (struct index **) malloc(idx_sz * idx_count);
- if (new_indexes == NULL) {
- diag_set(OutOfMemory, idx_sz * idx_count,
- "malloc", "new_indexes");
- return;
- }
- memcpy(new_indexes, space->index, i * idx_sz);
- memcpy(new_indexes + i, space->index + i + 1,
- idx_sz * (idx_count - i));
- free(space->index);
- space->index = new_indexes;
- space->index_count--;
- break;
- }
+ if (space->index[i]->def->iid == iid)
+ to_delete = space->index[i];
+ else if (new_index_id_max < space->index[i]->def->iid)
+ new_index_id_max = space->index[i]->def->iid;
}
+ /*
+ * Allocate new chunk with size reduced by 1 slot. Copy
+ * all indexes to that chunk except for one to be deleted.
+ */
+ assert(to_delete != NULL);
+ uint32_t new_index_count = space->index_count - 1;
+ uint32_t size = (new_index_count + new_index_id_max + 1) *
+ sizeof(struct index *);
+ struct index **new_index_map = (struct index **) malloc(size);
+ if (new_index_map == NULL) {
+ diag_set(OutOfMemory, size, "malloc", "new_index_map");
+ return -1;
+ }
+ /* Nullify gaps. */
+ memset(new_index_map, 0,
+ sizeof(struct index *) * (new_index_id_max + 1));
+ struct index **new_indexes = new_index_map + new_index_id_max + 1;
+ for (uint32_t i = 0, j = 0; i < space->index_count; ++i) {
+ struct index *idx = space->index[i];
+ if (idx == to_delete)
+ continue;
+ new_index_map[idx->def->iid] = space->index_map[idx->def->iid];
+ new_indexes[j++] = idx;
+ }
+ index_def_delete(to_delete->def);
+ free(to_delete);
+ free(space->index_map);
+ space->index_map = new_index_map;
+ space->index = new_indexes;
+ space->index_count--;
+ space->index_id_max = new_index_id_max;
struct session *user_session = current_session();
user_session->sql_flags |= SQLITE_InternChanges;
+ return 0;
}
/*
@@ -266,16 +278,11 @@ table_delete(struct sqlite3 *db, struct Table *tab)
goto skip_index_delete;
/* Delete all indices associated with this table. */
for (uint32_t i = 0; i < tab->space->index_count; ++i) {
- /*
- * These indexes are just wrapper for
- * index_def's, so it makes no sense to call
- * index_delete().
- */
struct index *idx = tab->space->index[i];
- free(idx->def);
+ index_def_delete(idx->def);
free(idx);
}
- free(tab->space->index);
+ free(tab->space->index_map);
free(tab->space);
skip_index_delete:
assert(tab->def != NULL);
@@ -362,14 +369,6 @@ sqlite3CheckIdentifierName(Parse *pParse, char *zName)
return SQLITE_OK;
}
-struct index *
-sql_table_primary_key(const struct Table *tab)
-{
- if (tab->space->index_count == 0 || tab->space->index[0]->def->iid != 0)
- return NULL;
- return tab->space->index[0];
-}
-
/**
* Create and initialize a new SQL Table object.
* All memory except table object itself is allocated on region.
@@ -812,7 +811,7 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
int nTerm;
if (pTab == 0)
goto primary_key_exit;
- if (sql_table_primary_key(pTab) != NULL) {
+ if (space_index(pTab->space, 0) != NULL) {
sqlite3ErrorMsg(pParse,
"table \"%s\" has more than one primary key",
pTab->def->name);
@@ -872,7 +871,7 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
goto primary_key_exit;
}
- struct index *pk = sql_table_primary_key(pTab);
+ struct index *pk = space_index(pTab->space, 0);
assert(pk != NULL);
struct key_def *pk_key_def = pk->def->key_def;
for (uint32_t i = 0; i < pk_key_def->part_count; i++) {
@@ -1146,11 +1145,13 @@ getNewSpaceId(Parse * pParse)
* @param idx_def Definition of index under construction.
* @param pk_def Definition of primary key index.
* @param space_id_reg Register containing generated space id.
+ * @param index_id_reg Register containing generated index id.
*/
static void
vdbe_emit_create_index(struct Parse *parse, struct space_def *def,
const struct index_def *idx_def,
- const struct index_def *pk_def, int space_id_reg)
+ const struct index_def *pk_def, int space_id_reg,
+ int index_id_reg)
{
struct Vdbe *v = sqlite3GetVdbe(parse);
int entry_reg = ++parse->nMem;
@@ -1187,10 +1188,11 @@ vdbe_emit_create_index(struct Parse *parse, struct space_def *def,
} else {
/*
* An existing table is being modified;
- * space_id_reg is register, but iid is literal.
+ * space_id_reg is literal, index_id_reg is
+ * register.
*/
sqlite3VdbeAddOp2(v, OP_Integer, space_id_reg, entry_reg);
- sqlite3VdbeAddOp2(v, OP_SCopy, idx_def->iid, entry_reg + 1);
+ sqlite3VdbeAddOp2(v, OP_SCopy, index_id_reg, entry_reg + 1);
}
sqlite3VdbeAddOp4(v, OP_String8, 0, entry_reg + 2, 0,
sqlite3DbStrDup(parse->db, idx_def->name),
@@ -1612,7 +1614,7 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
}
if (!p->def->opts.is_view) {
- if (sql_table_primary_key(p) == NULL) {
+ if (space_index(p->space, 0) == NULL) {
sqlite3ErrorMsg(pParse,
"PRIMARY KEY missing on table %s",
p->def->name);
@@ -1686,12 +1688,12 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
createSpace(pParse, reg_space_id, stmt);
/* Indexes aren't required for VIEW's.. */
if (!p->def->opts.is_view) {
- struct index *pk = sql_table_primary_key(p);
+ struct index *pk = space_index(p->space, 0);
for (uint32_t i = 0; i < p->space->index_count; ++i) {
struct index *idx = p->space->index[i];
- idx->def->iid = i;
vdbe_emit_create_index(pParse, p->def, idx->def,
- pk->def, reg_space_id);
+ pk->def, reg_space_id,
+ idx->def->iid);
}
}
@@ -1739,7 +1741,7 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
}
fk->parent_id = reg_space_id;
} else if (fk_parse->is_self_referenced) {
- struct index *pk = sql_table_primary_key(p);
+ struct index *pk = space_index(p->space, 0);
if (pk->def->key_def->part_count != fk->field_count) {
diag_set(ClientError, ER_CREATE_FK_CONSTRAINT,
fk->name, "number of columns in "\
@@ -2480,30 +2482,39 @@ getNewIid(Parse * pParse, int iSpaceId, int iCursor)
/**
* Add new index to table's indexes list.
- * We follow convention that PK comes first in list.
- *
- * @param index Index to be added to list.
* @param tab Table to which belongs given index.
+ * @param index Index to be added to list.
*/
static void
table_add_index(struct Table *tab, struct index *index)
{
- uint32_t idx_count = tab->space->index_count;
- size_t indexes_sz = sizeof(struct index *) * (idx_count + 1);
- struct index **idx = (struct index **) realloc(tab->space->index,
- indexes_sz);
- if (idx == NULL) {
- diag_set(OutOfMemory, indexes_sz, "realloc", "idx");
+ struct space *space = tab->space;
+ uint32_t new_index_count = space->index_count + 1;
+ uint32_t new_index_id_max = MAX(space->index_id_max, index->def->iid);
+ uint32_t size =
+ (new_index_id_max + 1 + new_index_count) * sizeof(index);
+ struct index **new_index_map = (struct index **) malloc(size);
+ if (new_index_map == NULL) {
+ diag_set(OutOfMemory, size, "malloc", "new_index_map");
return;
}
- tab->space->index = idx;
- /* Make sure that PK always comes as first member. */
- if (index->def->iid == 0 && idx_count != 0) {
- struct index *tmp = tab->space->index[0];
- tab->space->index[0] = index;
- index = tmp;
- }
- tab->space->index[tab->space->index_count++] = index;
+ struct index **new_index = new_index_map + new_index_id_max + 1;
+ memcpy(new_index_map, space->index_map,
+ sizeof(index) * (space->index_id_max + 1));
+ memcpy(new_index, space->index, sizeof(index) * space->index_count);
+ new_index_map[index->def->iid] = index;
+ /*
+ * Make sure that PK always comes as first member in order
+ * to prefer it among other indexes when possible.
+ */
+ if (index->def->iid == 0 && space->index_count > 0)
+ SWAP(index, new_index[0]);
+ new_index[new_index_count - 1] = index;
+ free(space->index_map);
+ space->index_map = new_index_map;
+ space->index = new_index;
+ space->index_count = new_index_count;
+ space->index_id_max = new_index_id_max;
}
/**
@@ -2798,7 +2809,11 @@ sql_create_index(struct Parse *parse, struct Token *token,
* (for the simplicity sake we set it to 1), but PK
* still must have iid == 0.
*/
- uint32_t iid = idx_type != SQL_INDEX_TYPE_CONSTRAINT_PK;
+ uint32_t iid;
+ if (idx_type != SQL_INDEX_TYPE_CONSTRAINT_PK)
+ iid = table->space->index_id_max + 1;
+ else
+ iid = 0;
if (index_fill_def(parse, index, table, iid, name, strlen(name),
col_list, idx_type, sql_stmt) != 0)
goto exit_create_index;
@@ -2858,6 +2873,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
if (table == parse->pNewTable) {
for (uint32_t i = 0; i < table->space->index_count; ++i) {
struct index *existing_idx = table->space->index[i];
+ uint32_t iid = existing_idx->def->iid;
struct key_def *key_def = index->def->key_def;
struct key_def *exst_key_def =
existing_idx->def->key_def;
@@ -2882,7 +2898,9 @@ sql_create_index(struct Parse *parse, struct Token *token,
constraint_is_named(existing_idx->def->name);
/* CREATE TABLE t(a, UNIQUE(a), PRIMARY KEY(a)). */
if (idx_type == SQL_INDEX_TYPE_CONSTRAINT_PK &&
- existing_idx->def->iid != 0 && !is_named) {
+ iid != 0 && !is_named) {
+ table->space->index_map[0] = existing_idx;
+ table->space->index_map[iid] = NULL;
existing_idx->def->iid = 0;
goto exit_create_index;
}
@@ -2938,9 +2956,9 @@ sql_create_index(struct Parse *parse, struct Token *token,
space_id = table->def->id;
index_id = getNewIid(parse, space_id, cursor);
sqlite3VdbeAddOp1(vdbe, OP_Close, cursor);
- struct index *pk = sql_table_primary_key(table);
+ struct index *pk = space_index(table->space, 0);
vdbe_emit_create_index(parse, table->def, index->def, pk->def,
- space_id);
+ space_id, index_id);
/* Consumes sql_stmt. */
first_schema_col =
vdbe_emit_index_schema_record(parse, index->def->name,
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 0f285cc8b..8ec198d88 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -351,7 +351,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
* key.
*/
key_len = 0;
- struct index *pk = sql_table_primary_key(table);
+ struct index *pk = space_index(space, 0);
const char *zAff = is_view ? NULL :
sql_space_index_affinity_str(parse->db,
space->def,
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index dcb57e9b3..f26372ce7 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -3140,7 +3140,7 @@ sqlite3ExprCodeIN(Parse * pParse, /* Parsing and code generating context */
struct Table *tab = src_list->a[0].pTab;
assert(tab != NULL);
- struct index *pk = sql_table_primary_key(tab);
+ struct index *pk = space_index(tab->space, 0);
assert(pk != NULL);
uint32_t fieldno = pk->def->key_def->parts[0].fieldno;
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 9a7fdffbb..cde3bc34c 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -960,7 +960,7 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct Table *tab,
* FIXME: should be removed after introducing
* strict typing.
*/
- struct index *pk = sql_table_primary_key(tab);
+ struct index *pk = space_index(tab->space, 0);
uint32_t part_count = pk->def->key_def->part_count;
if (part_count == 1) {
uint32_t fieldno = pk->def->key_def->parts[0].fieldno;
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 3ba7ad022..c5b664414 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -443,7 +443,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
break;
struct space *space = space_cache_find(table->def->id);
struct space_def *def = space->def;
- struct index *pk = sql_table_primary_key(table);
+ struct index *pk = space_index(space, 0);
pParse->nMem = 6;
if (def->opts.is_view) {
const char *sql = table->def->opts.sql;
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 02732b59c..9f1791a6e 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3343,12 +3343,6 @@ int sqlite3ColumnsFromExprList(Parse *parse, ExprList *expr_list, Table *table);
void sqlite3SelectAddColumnTypeAndCollation(Parse *, Table *, Select *);
Table *sqlite3ResultSetOfSelect(Parse *, Select *);
-/**
- * Return the PRIMARY KEY index of a table.
- */
-struct index *
-sql_table_primary_key(const struct Table *tab);
-
void sqlite3StartTable(Parse *, Token *, int);
void sqlite3AddColumn(Parse *, Token *, Token *);
@@ -3667,8 +3661,11 @@ void sqlite3UnlinkAndDeleteTable(sqlite3 *, const char *);
*
* @param space Space which index belongs to.
* @param iid Id of index to be deleted.
+ *
+ * @retval 0 Success.
+ * @retval -1 Error.
*/
-void
+int
sql_space_index_delete(struct space *space, uint32_t iid);
char *sqlite3NameFromToken(sqlite3 *, Token *);
diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
index f3e65303b..d2475bbc8 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -207,7 +207,6 @@ fkey_encode_links(struct region *region, const struct fkey_def *def, int type,
uint32_t *size);
/**
-<<<<<<< HEAD
* Encode index parts of given foreign key constraint into
* MsgPack on @region.
* @param region Region to use.
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 076b28a94..852be6fba 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -146,7 +146,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
/* Allocate cursor on primary index. */
int pk_cursor = pParse->nTab++;
pTabList->a[0].iCursor = pk_cursor;
- struct index *pPk = sql_table_primary_key(pTab);
+ struct index *pPk = space_index(pTab->space, 0);
i = sizeof(int) * def->field_count;
aXRef = (int *) region_alloc(&pParse->region, i);
if (aXRef == NULL) {
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index b956726e4..21136688c 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4746,7 +4746,10 @@ case OP_DropTable: {
* This is called after an index is dropped from Tarantool DD.
*/
case OP_DropIndex: {
- sql_space_index_delete(pOp->p4.space, pOp->p1);
+ if (sql_space_index_delete(pOp->p4.space, pOp->p1) != 0) {
+ rc = SQL_TARANTOOL_ERROR;
+ goto abort_due_to_error;
+ }
break;
}
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 3e50f9b89..ef70442b3 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -1089,7 +1089,7 @@ valueNew(sqlite3 * db, struct ValueNewStat4Ctx *p)
uint32_t part_count = idx->key_def->part_count;
int nByte = sizeof(Mem) * part_count +
- ROUND8(sizeof(UnpackedRecord));
+ ROUND8(sizeof(UnpackedRecord));
pRec = (UnpackedRecord *) sqlite3DbMallocZero(db,
nByte);
if (pRec == NULL)
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 1aa858ac1..e2aaca3c3 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -1351,7 +1351,7 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about t
Expr *pAndExpr = 0; /* An ".. AND (...)" expression */
Table *pTab = pTabItem->pTab;
struct key_def *pk_key_def =
- sql_table_primary_key(pTab)->def->key_def;
+ space_index(pTab->space, 0)->def->key_def;
uint32_t pk_part_count = pk_key_def->part_count;
pTerm = pLoop->aLTerm[0];
diff --git a/test/sql-tap/gh2130-index-refer-table.test.lua b/test/sql-tap/gh2130-index-refer-table.test.lua
index 3a6064cb9..b5fc1106d 100755
--- a/test/sql-tap/gh2130-index-refer-table.test.lua
+++ b/test/sql-tap/gh2130-index-refer-table.test.lua
@@ -69,16 +69,4 @@ test:do_execsql_test(
-- <index-1.5>
})
--- This part of test is banned in scope of #2174
--- test:do_execsql_test(
--- "index-1.6",
--- [[
--- REINDEX t1ix1 ON t1;
--- ]],
--- {
--- -- <index-1.6>
---
--- -- <index-1.6>
--- })
-
test:finish_test()
diff --git a/test/sql-tap/index1.test.lua b/test/sql-tap/index1.test.lua
index fdce2683c..90ce4b232 100755
--- a/test/sql-tap/index1.test.lua
+++ b/test/sql-tap/index1.test.lua
@@ -119,67 +119,6 @@ test:do_test(
-- </index-2.2>
})
--- MUST_WORK_TEST REINDEX and integrity_check
-if (0 > 0)
- then
- -- Try creating a bunch of indices on the same table
- --
- local r = {}
- for i = 1, 99, 1 do
- table.insert(r,string.format("index%02d", i))
- end
- test:do_test(
- "index-3.1",
- function()
- test:execsql("CREATE TABLE test1(f1 int primary key, f2 int, f3 int, f4 int, f5 int)")
- for i = 1, 99, 1 do
- local sql = string.format("CREATE INDEX %s ON test1(f%s)", string.format("index%02d", i), (i%5)+1)
- test:execsql(sql)
- end
- return test:execsql [[SELECT name FROM sqlite_master
- WHERE type='index' AND tbl_name='test1'
- ORDER BY name]]
- end, {
- -- <index-3.1>
- r
- -- </index-3.1>
- })
-
- X(104, "X!cmd", [=[["integrity_check","index-3.2.1"]]=])
- test:do_execsql_test(
- "index-3.2.2",
- [[
- REINDEX
- ]], {
- -- <index-3.2.2>
-
- -- </index-3.2.2>
- })
-
-
-
- --X(110, "X!cmd", [=[["integrity_check","index-3.2.3"]]=])
- -- Verify that all the indices go away when we drop the table.
- --
- test:do_test(
- "index-3.3",
- function()
- test:execsql "DROP TABLE test1"
- return test:execsql [[SELECT name FROM sqlite_master
- WHERE type='index' AND tbl_name='test1'
- ORDER BY name]]
- end, {
- -- <index-3.3>
-
- -- </index-3.3>
- })
-
- -- Create a table and insert values into that table. Then create
- -- an index on that table. Verify that we can select values
- -- from the table correctly using the index
- -- Note that the index names index9 and indext are chosen because
- -- they both have the same hash.
-end
test:do_test(
"index-4.1",
function()
@@ -1017,7 +956,7 @@ test:do_execsql_test(
SELECT "_index"."name" FROM "_index" JOIN "_space" WHERE "_index"."id" = "_space"."id" AND "_space"."name"='T7';
]], {
-- <index-17.1>
- "pk_unnamed_T7_3", "unique_unnamed_T7_2", "unique_unnamed_T7_1"
+ "pk_unnamed_T7_3", "unique_unnamed_T7_1", "unique_unnamed_T7_2"
-- </index-17.1>
})
diff --git a/test/sql-tap/index6.test.lua b/test/sql-tap/index6.test.lua
index 069623f66..af18e89d4 100755
--- a/test/sql-tap/index6.test.lua
+++ b/test/sql-tap/index6.test.lua
@@ -106,13 +106,6 @@ test:plan(14)
-- SELECT idx, stat FROM sqlite_stat1 ORDER BY idx;
-- }
-- } {{} 15 t1a {10 1} t1b {8 1} ok}
--- do_test index6-1.14 {
--- execsql {
--- REINDEX;
--- ANALYZE;
--- SELECT idx, stat FROM sqlite_stat1 ORDER BY idx;
--- }
--- } {{} 15 t1a {10 1} t1b {8 1} ok}
-- do_test index6-1.15 {
-- execsql {
-- CREATE INDEX t1c ON t1(c);
diff --git a/test/sql-tap/index7.test.lua b/test/sql-tap/index7.test.lua
index 4f9070813..7d4a54723 100755
--- a/test/sql-tap/index7.test.lua
+++ b/test/sql-tap/index7.test.lua
@@ -126,14 +126,6 @@ end
-- PRAGMA integrity_check;
-- }
-- } {t1 {15 1} t1a {10 1} t1b {8 1} ok}
--- do_test index7-1.14 {
--- execsql {
--- REINDEX;
--- ANALYZE;
--- SELECT idx, stat FROM sqlite_stat1 ORDER BY idx;
--- PRAGMA integrity_check;
--- }
--- } {t1 {15 1} t1a {10 1} t1b {8 1} ok}
-- do_test index7-1.15 {
-- execsql {
-- CREATE INDEX t1c ON t1(c);
diff --git a/test/sql-tap/insert1.test.lua b/test/sql-tap/insert1.test.lua
index 750732d37..cfca0025f 100755
--- a/test/sql-tap/insert1.test.lua
+++ b/test/sql-tap/insert1.test.lua
@@ -209,11 +209,6 @@ end, {
-- do_test insert-3.4 {
-- execsql {SELECT * FROM test2 WHERE f1=22 AND f2=-4.44}
-- } {22 -4.44 hi abc-123 wham}
--- ifcapable {reindex} {
--- do_test insert-3.5 {
--- execsql REINDEX
--- } {}
--- }
-- integrity_check insert-3.5
-- Test of expressions in the VALUES clause
--
@@ -395,11 +390,6 @@ test:do_execsql_test("insert-4.7", [[
-- SELECT * FROM t1 WHERE b=3;
-- }
-- } {}
- -- ifcapable {reindex} {
- -- do_test insert-6.5 {
- -- execsql REINDEX
- -- } {}
- -- }
-- do_test insert-6.6 {
-- execsql {
-- DROP TABLE t1;
diff --git a/test/sql-tap/misc3.test.lua b/test/sql-tap/misc3.test.lua
index 92f8210c9..dc1545f8f 100755
--- a/test/sql-tap/misc3.test.lua
+++ b/test/sql-tap/misc3.test.lua
@@ -443,46 +443,6 @@ test:do_test(
-- </misc3-6.3>
})
--- Do some additional EXPLAIN operations to exercise the displayP4 logic.
--- This part of test is disabled in scope of #2174
--- test:do_test(
--- "misc3-6.10",
--- function()
--- local x = test:execsql([[
--- CREATE TABLE ex1(
--- id PRIMARY KEY,
--- a INTEGER DEFAULT 54321,
--- b TEXT DEFAULT "hello",
--- c REAL DEFAULT 3.1415926
--- );
--- CREATE UNIQUE INDEX ex1i1 ON ex1(a);
--- EXPLAIN REINDEX;
--- ]])
--- x = json.encode(x)
--- return string.find(x, "\"SorterCompare\",%d+,%d+,%d+") > 0
--- end, true)
---
--- test:do_test(
--- "misc3-6.11-utf8",
--- function()
--- local x = test:execsql([[
--- EXPLAIN SELECT a+123456789012, b*4.5678, c FROM ex1 ORDER BY +a, b DESC
--- ]])
--- x = json.encode(x)
--- local y = {}
--- table.insert(y, string.find(x, "123456789012")>0)
--- table.insert(y, string.find(x, "4.5678")>0)
--- table.insert(y, string.find(x, "hello")>0)
--- table.insert(y, string.find(x, "-B")>0)
--- return y
--- end, {
--- -- <misc3-6.11-utf8>
--- 1, 1, 1, 1
--- -- </misc3-6.11-utf8>
--- })
-
-
-
-- MUST_WORK_TEST autoincrement for pk
if (0 > 0) then
-- Ticket #640: vdbe stack overflow with a LIMIT clause on a SELECT inside
diff --git a/test/sql-tap/reindex.test.lua b/test/sql-tap/reindex.test.lua
deleted file mode 100755
index d4f8c8188..000000000
--- a/test/sql-tap/reindex.test.lua
+++ /dev/null
@@ -1,36 +0,0 @@
-#!/usr/bin/env tarantool
-test = require("sqltester")
-test:plan(3)
-
-
-test:execsql("CREATE TABLE t1(a INT PRIMARY KEY);")
-test:execsql("CREATE INDEX t1ix1 ON t1(a)")
-
-test:do_execsql_test(
- "reindex-1.1",
- "REINDEX t1ix1 ON t1",
- {
- -- <reindex-1.1>
-
- -- <reindex-1.1>
- })
-
-test:do_catchsql_test(
- "reindex-1.2",
- "REINDEX t1ix2 ON t1",
- {
- -- <reindex-1.1>
- 1, "unable to identify the object to be reindexed"
- -- <reindex-1.1>
- })
-
-test:do_catchsql_test(
- "reindex-1.3",
- "REINDEX t1ix1 ON t3",
- {
- -- <reindex-1.1>
- 1, "no such table: T3"
- -- <reindex-1.1>
- })
-
-test:finish_test()
diff --git a/test/sql-tap/suite.ini b/test/sql-tap/suite.ini
index 0637cffc1..2455f5e8d 100644
--- a/test/sql-tap/suite.ini
+++ b/test/sql-tap/suite.ini
@@ -1,8 +1,6 @@
[default]
core = app
description = Database tests with #! using TAP
-disabled =
- reindex.test.lua ; This test is banned in scope of #2174
lua_libs = lua/sqltester.lua ../sql/lua/sql_tokenizer.lua ../box/lua/identifier.lua
is_parallel = True
release_disabled = debug_mode_only.test.lua
More information about the Tarantool-patches
mailing list