[tarantool-patches] Re: [PATCH 2/7] sql: remove SQLite original struct Index

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch! See 16 comments below and a
commit on the branch.

On 23/08/2018 19:55, Nikita Pettik wrote:
> As a part of SQL DD integration it is required to substitute SQLite
> structure representing index with one from Tarantool internals.
> To make this happen, lets add surrogate space to Table, which will
> hold array of indexes. Those indexes are not real copies from Tarantool
> core, but contain only index_def, since only def is useful during query
> compilation.
> 
> Note that in new implementation indexes are held as array and added to
> that array in straight order. In SQLite indexes are arranged in list and
> added to the head. Hence, the order of indexes is reversed. It results
> in different query plans: if planner must make choice of two equal
> indexes, it chooses simply first one. Due to this change, some tests are
> fixed.
> 
> Part of #3561
> ---
>   src/box/sql.c                            |  56 +--
>   src/box/sql/analyze.c                    |  69 ++--
>   src/box/sql/build.c                      | 689 ++++++++++---------------------
>   src/box/sql/delete.c                     |  14 +-
>   src/box/sql/expr.c                       | 120 +-----
>   src/box/sql/insert.c                     |  92 ++---
>   src/box/sql/parse.y                      |   8 -
>   src/box/sql/pragma.c                     | 188 ++++-----
>   src/box/sql/prepare.c                    |   7 +-
>   src/box/sql/select.c                     |  17 +-
>   src/box/sql/sqliteInt.h                  |  79 ++--
>   src/box/sql/tarantoolInt.h               |  34 +-
>   src/box/sql/update.c                     |   3 +-
>   src/box/sql/vdbe.c                       |  12 +-
>   src/box/sql/vdbe.h                       |   6 +-
>   src/box/sql/vdbeaux.c                    |   6 +-
>   src/box/sql/vdbemem.c                    |  28 +-
>   src/box/sql/where.c                      | 381 ++++++++---------
>   src/box/sql/whereInt.h                   |   8 +-
>   src/box/sql/wherecode.c                  | 167 +++-----
>   test/sql-tap/analyze3.test.lua           |   2 +-
>   test/sql-tap/analyze7.test.lua           |   8 +-
>   test/sql-tap/analyze9.test.lua           |  10 +-
>   test/sql-tap/analyzeF.test.lua           |   2 +-
>   test/sql-tap/eqp.test.lua                |  14 +-
>   test/sql-tap/gh-2996-indexed-by.test.lua |  10 +-
>   test/sql-tap/lua-tables.test.lua         |   8 +-
>   27 files changed, 807 insertions(+), 1231 deletions(-)
> 
> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index 74f5ae827..abed9d31b 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -819,23 +817,27 @@ analyzeOneTable(Parse * pParse,	/* Parser context */
>   	pParse->nTab = MAX(pParse->nTab, iTab);
>   	sqlite3OpenTable(pParse, iTabCur, pTab, OP_OpenRead);
>   	sqlite3VdbeLoadString(v, regTabname, pTab->def->name);
> -
> -	for (pIdx = pTab->pIndex; pIdx; pIdx = pIdx->pNext) {
> +	/*
> +	 * 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);
> +	for (uint32_t j = 0; j < space->index_count; ++j) {
> +		struct index *idx = pTab->space->index[j];
>   		int addrRewind;	/* Address of "OP_Rewind iIdxCur" */
>   		int addrNextRow;	/* Address of "next_row:" */
>   		const char *idx_name;	/* Name of the index */
>   
> -		if (pOnlyIdx && pOnlyIdx != pIdx)
> -			continue;
>   		/* Primary indexes feature automatically generated
>   		 * names. Thus, for the sake of clarity, use
>   		 * instead more familiar table name.
>   		 */
> -		if (sql_index_is_primary(pIdx))
> +		if (idx->def->iid == 0)
>   			idx_name = pTab->def->name;
>   		else
> -			idx_name = pIdx->def->name;
> -		int part_count = pIdx->def->key_def->part_count;
> +			idx_name = idx->def->name;
> +		int part_count = idx->def->key_def->part_count;

1. Below you have check 'if (part_count > 0)', but it is
always true, it is not?

>   
>   		/* Populate the register containing the index name. */
>   		sqlite3VdbeLoadString(v, regIdxname, idx_name);
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 47fa7c305..79dc46592 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -151,55 +151,46 @@ sqlite3LocateIndex(sqlite3 * db, const char *zName, const char *zTable)
>   
>   	if (pTab == NULL)
>   		return NULL;
> -	for (struct Index *idx = pTab->pIndex; idx != NULL; idx = idx->pNext) {
> +	for (uint32_t i = 0; i < pTab->space->index_count; ++i) {
> +		struct index *idx = pTab->space->index[i];
>   		if (strcmp(zName, idx->def->name) == 0)
>   			return idx;
>   	}
>   	return NULL;
>   }
>   
> -/*
> - * Reclaim the memory used by an index
> - */
> -static void
> -freeIndex(sqlite3 * db, Index * p)
> -{
> -	if (p->def != NULL)
> -		index_def_delete(p->def);
> -	sqlite3DbFree(db, p);
> -}
> -
> -/*
> - * For the index called zIdxName which is found in the database,
> - * unlike that index from its Table then remove the index from
> - * the index hash table and free all memory structures associated
> - * with the index.
> - */
>   void
> -sqlite3UnlinkAndDeleteIndex(sqlite3 * db, Index * pIndex)
> +sql_space_index_delete(struct space *space, uint32_t iid)
>   {
> -	assert(pIndex != 0);
> -
> -	struct session *user_session = current_session();
> -	if (ALWAYS(pIndex)) {
> -		if (pIndex->pTable->pIndex == pIndex) {
> -			pIndex->pTable->pIndex = pIndex->pNext;
> -		} else {
> -			Index *p;
> -			/* Justification of ALWAYS();  The index must be on the list of
> -			 * indices.
> -			 */
> -			p = pIndex->pTable->pIndex;
> -			while (ALWAYS(p) && p->pNext != pIndex) {
> -				p = p->pNext;
> -			}
> -			if (ALWAYS(p && p->pNext == pIndex)) {
> -				p->pNext = pIndex->pNext;
> +	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.

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.

> +		/*
> +		 * 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) {
> +			free(idx->def);

3. idx->def->key_def leaks. cmp_def as well. Please,
use index_def_delete. Same in other places where you
delete index_def (whereLoopClearUnion, etc).

> +			free(idx);
> +			size_t idx_sz = sizeof(struct index *);
> +			uint32_t idx_count = --space->index_count;

4. If malloc below fails, index_count will remain decremented.

> +			struct index **new_idexes =
> +				(struct index **) malloc(idx_sz * idx_count);

5. Typo: 'idexes'.

> +			if (new_idexes == NULL) {
> +				diag_set(OutOfMemory, idx_sz * idx_count,
> +					 "malloc", "new_indexes");
> +				return;
>   			}
> +			memcpy(new_idexes, space->index, i * idx_sz);
> +			memcpy(new_idexes + i, space->index + i + 1,
> +			       idx_sz * (idx_count - i));
> +			free(space->index);
> +			space->index = new_idexes;
> +			break;
>   		}
> -		freeIndex(db, pIndex);
>   	}
> -
> +	struct session *user_session = current_session();
>   	user_session->sql_flags |= SQLITE_InternChanges;
>   }
>   
> @@ -259,38 +250,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.
> + * Table.
>    *
> - * 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.
> - *
> - * 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);
> +		free(idx);
> +	}
> +	free(tab->space);
> +	free(tab->space->index);

6. Use after free (tab->space is freed here).

> +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
> @@ -368,16 +360,12 @@ sqlite3CheckIdentifierName(Parse *pParse, char *zName)
>   	return SQLITE_OK;
>   }
>   
> -/*
> - * Return the PRIMARY KEY index of a table
> - */
> -Index *
> -sqlite3PrimaryKeyIndex(Table * pTab)
> +struct index *
> +sql_table_primary_key(const struct Table *tab)
>   {
> -	Index *p;
> -	for (p = pTab->pIndex; p != NULL && !sql_index_is_primary(p);
> -	     p = p->pNext);
> -	return p;
> +	if (tab->space->index_count == 0 || tab->space->index[0]->def->iid != 0)
> +		return NULL;
> +	return tab->space->index[0];

7. If you had index_map, you could do it like this:

     if (tab->space-index_count == 0)
         return NULL;
     return tab->space->index_map[0];

No checking for index[0]->iid == 0, it looks very
strange. Likewise a primary index could have id != 0.

You can allocate index_map in the same memory block
as index array.

>   }
>   
>   /**
> @@ -2431,84 +2408,6 @@ sql_drop_foreign_key(struct Parse *parse_context, struct SrcList *table,
>   		vdbe_emit_fkey_drop(parse_context, constraint_name, child_id);
>   }
>   
> -/*
> - * Generate code that will erase and refill index *pIdx.  This is
> - * used to initialize a newly created index or to recompute the
> - * content of an index in response to a REINDEX command.
> - */
> -static void
> -sqlite3RefillIndex(Parse * pParse, Index * pIndex)

8. Reindex still has a plenty of mentions over the code,
including the parser and commented tests. What are you going
to do with that? I propose to remove it from all the places.

> @@ -2558,16 +2457,24 @@ getNewIid(Parse * pParse, int iSpaceId, int iCursor)
> +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, "malloc", "idx");
> +		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;

9. Do not care about index array. You should only
care about index_map - here the pk should be on the
first place. So this whole 'if' can be replaced with
a single tab->space->index_map[iid] = index.

>   	}
> +	tab->space->index[tab->space->index_count++] = index;
>   }
>   
>   /**
> @@ -2751,10 +2652,10 @@ sql_create_index(struct Parse *parse, struct Token *token,
>   	 *    auto-index name will be generated.
>   	 */
>   	if (token != NULL) {
> +		assert(token->z != NULL);
>   		name = sqlite3NameFromToken(db, token);
>   		if (name == NULL)
>   			goto exit_create_index;
> -		assert(token->z != NULL);

10. Noise diff hunk.

>   		if (sqlite3LocateIndex(db, name, table->def->name) != NULL) {
>   			if (!if_not_exist) {
>   				sqlite3ErrorMsg(parse,
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 7780bf749..9220d34e1 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c> @@ -1353,12 +1352,6 @@ xferOptimization(Parse * pParse,	/* Parser context */
>   		sqlite3VdbeJumpHere(v, addr1);
>   	}
>   
> -	for (pDestIdx = pDest->pIndex; pDestIdx; pDestIdx = pDestIdx->pNext) {
> -		for (pSrcIdx = pSrc->pIndex; ALWAYS(pSrcIdx);
> -		     pSrcIdx = pSrcIdx->pNext) {
> -			if (xferCompatibleIndex(pDestIdx, pSrcIdx))
> -				break;
> -		}

11. Why are you sure that pDestIdx and pSrcIdx are xfercompatible
now? And the indentation is broken.

As I see, pDestIdx and pSrcIdx above are checked for xfer in a
cycle, but it is not stopped once they are found.

>   		assert(pSrcIdx);
>   		struct space *src_space =
>   			space_by_id(pSrc->def->id);
> diff --git a/src/box/sql/where.c b/src/box/sql/where.c
> index a4a1c456f..d8a2c1a79 100644
> --- a/src/box/sql/where.c
> +++ b/src/box/sql/where.c
> @@ -1741,12 +1740,11 @@ whereLoopInit(WhereLoop * p)
>    * Clear the WhereLoop.u union.  Leave WhereLoop.pLTerm intact.
>    */
>   static void
> -whereLoopClearUnion(sqlite3 * db, WhereLoop * p)
> +whereLoopClearUnion(WhereLoop * p)
>   {
> -	if ((p->wsFlags & WHERE_AUTO_INDEX) != 0 &&
> -	    (p->wsFlags & WHERE_AUTO_INDEX) != 0 && p->pIndex != 0) {
> -		sqlite3DbFree(db, p->pIndex);
> -		p->pIndex = 0;
> +	if ((p->wsFlags & WHERE_AUTO_INDEX) != 0) {

12. I put here assert(p->index_def->key_def == NULL)
and it did not fire. Then I tried assert(false) and the
result was the same. So this code is not tested at all.
Can you fix it?

> +		free(p->index_def);
> +		p->index_def = NULL;
>   	}
> @@ -2999,13 +2975,15 @@ whereLoopAddBtree(WhereLoopBuilder * pBuilder,	/* WHERE clause information */
>   		/* If there was an INDEXED BY clause, then only that one index is
>   		 * considered.
>   		 */
> -		if (pSrc->pIBIndex)
> +		if (pSrc->pIBIndex != NULL)
> +			break;
> +		if (fake_index != NULL)
>   			break;

13. This 'break' makes no sense. You already
limited loop iterations with 1 for fake_index != NULL.
Either break, or limit 'for'.

> +
>   	}
> -	if (fake_index.def != NULL)
> -	{
> -		free(fake_index.def->opts.stat->tuple_log_est);
> -		index_def_delete(fake_index.def);
> +	if (fake_index != NULL) {
> +		free(fake_index->opts.stat->tuple_log_est);

14. Just allocate fake_index->opts.stat like it
works in ordinary index_defs and use index_def_delete
only.

> +		index_def_delete(fake_index);
>   	}
>   	return rc;
>   }

15. Why index_is_unordered still exists? Why can you
get key_def->opts.stat->is_unordered right from SQL index_def?

> @@ -4651,10 +4628,10 @@ sqlite3WhereBegin(Parse * pParse,	/* The parser context */
>   			 *    It is something w/ defined space_def
>   			 *    and nothing else. Skip such loops.
>   			 */
> -			if (idx_def == NULL && pIx == NULL)
> +			if (idx_def == NULL)
>   				continue;
> -			bool is_primary = (pIx != NULL && sql_index_is_primary(pIx)) ||
> -					  (idx_def != NULL && (idx_def->iid == 0));
> +			bool is_primary = (idx_def != NULL &&
> +					   idx_def->iid == 0);

16. idx_def == NULL is filtered out one line above. The same about other
'idx_def !=/== NULL' below.

>   			if (is_primary
>   			    && (wctrlFlags & WHERE_OR_SUBCLAUSE) != 0) {
>   				/* This is one term of an OR-optimization using

Please, consider my fixes on the branch and here:

commit cccbcaa444453aba3373ef53febb83389d2a113d
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Mon Aug 27 17:07:53 2018 -0300

     Review fixes

diff --git a/src/box/key_def.c b/src/box/key_def.c
index 75eba79ec..e171e88fa 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -250,6 +250,8 @@ key_part_cmp(const struct key_part *parts1, uint32_t part_count1,
  		if (part1->coll != part2->coll)
  			return (uintptr_t) part1->coll <
  			       (uintptr_t) part2->coll ? -1 : 1;
+		if (part1->sort_order != part2->sort_order)
+			return part1->sort_order < part2->sort_order ? -1 : 1;
  		if (key_part_is_nullable(part1) != key_part_is_nullable(part2))
  			return key_part_is_nullable(part1) <
  			       key_part_is_nullable(part2) ? -1 : 1;
diff --git a/src/box/sql.c b/src/box/sql.c
index 61c766540..f85daf9bd 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -31,10 +31,6 @@
  #include <assert.h>
  #include "field_def.h"
  #include "sql.h"
-/*
- * Both Tarantool and SQLite codebases declare Index, hence the
- * workaround below.
- */
  #include "sql/sqliteInt.h"
  #include "sql/tarantoolInt.h"
  #include "sql/vdbeInt.h"
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index abed9d31b..33a3e1da8 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -918,68 +918,67 @@ analyzeOneTable(Parse * pParse,	/* Parser context */
  		sqlite3VdbeAddOp2(v, OP_Integer, 0, regChng);
  		addrNextRow = sqlite3VdbeCurrentAddr(v);
  
-		if (part_count > 0) {
-			int endDistinctTest = sqlite3VdbeMakeLabel(v);
-			int *aGotoChng;	/* Array of jump instruction addresses */
-			aGotoChng =
-			    sqlite3DbMallocRawNN(db, sizeof(int) * part_count);
-			if (aGotoChng == 0)
-				continue;
-
+		int endDistinctTest = sqlite3VdbeMakeLabel(v);
+		/* Array of jump instruction addresses. */
+		int *aGotoChng =
+			sqlite3DbMallocRawNN(db, sizeof(int) * part_count);
+		if (aGotoChng == NULL)
+			continue;
+		/*
+		 *  next_row:
+		 *   regChng = 0
+		 *   if( idx(0) != regPrev(0) ) goto chng_addr_0
+		 *   regChng = 1
+		 *   if( idx(1) != regPrev(1) ) goto chng_addr_1
+		 *   ...
+		 *   regChng = N
+		 *   goto endDistinctTest
+		 */
+		sqlite3VdbeAddOp0(v, OP_Goto);
+		addrNextRow = sqlite3VdbeCurrentAddr(v);
+		if (part_count == 1 && idx->def->opts.is_unique) {
  			/*
-			 *  next_row:
-			 *   regChng = 0
-			 *   if( idx(0) != regPrev(0) ) goto chng_addr_0
-			 *   regChng = 1
-			 *   if( idx(1) != regPrev(1) ) goto chng_addr_1
-			 *   ...
-			 *   regChng = N
-			 *   goto endDistinctTest
+			 * For a single-column UNIQUE index, once
+			 * we have found a non-NULL row, we know
+			 * that all the rest will be distinct, so
+			 * skip subsequent distinctness tests.
  			 */
-			sqlite3VdbeAddOp0(v, OP_Goto);
-			addrNextRow = sqlite3VdbeCurrentAddr(v);
-			if (part_count == 1 && idx->def->opts.is_unique) {
-				/* For a single-column UNIQUE index, once we have found a non-NULL
-				 * row, we know that all the rest will be distinct, so skip
-				 * subsequent distinctness tests.
-				 */
-				sqlite3VdbeAddOp2(v, OP_NotNull, regPrev,
-						  endDistinctTest);
-				VdbeCoverage(v);
-			}
-			struct key_part *part = idx->def->key_def->parts;
-			for (i = 0; i < part_count; ++i, ++part) {
-				struct coll *coll = part->coll;
-				sqlite3VdbeAddOp2(v, OP_Integer, i, regChng);
-				sqlite3VdbeAddOp3(v, OP_Column, iIdxCur,
-						  part->fieldno, regTemp);
-				aGotoChng[i] =
-				    sqlite3VdbeAddOp4(v, OP_Ne, regTemp, 0,
-						      regPrev + i, (char *)coll,
-						      P4_COLLSEQ);
-				sqlite3VdbeChangeP5(v, SQLITE_NULLEQ);
-				VdbeCoverage(v);
-			}
-			sqlite3VdbeAddOp2(v, OP_Integer, part_count, regChng);
-			sqlite3VdbeGoto(v, endDistinctTest);
+			sqlite3VdbeAddOp2(v, OP_NotNull, regPrev,
+					  endDistinctTest);
+			VdbeCoverage(v);
+		}
+		struct key_part *part = idx->def->key_def->parts;
+		for (i = 0; i < part_count; ++i, ++part) {
+			struct coll *coll = part->coll;
+			sqlite3VdbeAddOp2(v, OP_Integer, i, regChng);
+			sqlite3VdbeAddOp3(v, OP_Column, iIdxCur, part->fieldno,
+					  regTemp);
+			aGotoChng[i] = sqlite3VdbeAddOp4(v, OP_Ne, regTemp, 0,
+							 regPrev + i,
+							 (char *)coll,
+							 P4_COLLSEQ);
+			sqlite3VdbeChangeP5(v, SQLITE_NULLEQ);
+			VdbeCoverage(v);
+		}
+		sqlite3VdbeAddOp2(v, OP_Integer, part_count, regChng);
+		sqlite3VdbeGoto(v, endDistinctTest);
  
-			/*
-			 *  chng_addr_0:
-			 *   regPrev(0) = idx(0)
-			 *  chng_addr_1:
-			 *   regPrev(1) = idx(1)
-			 *  ...
-			 */
-			sqlite3VdbeJumpHere(v, addrNextRow - 1);
-			part = idx->def->key_def->parts;
-			for (i = 0; i < part_count; ++i, ++part) {
-				sqlite3VdbeJumpHere(v, aGotoChng[i]);
-				sqlite3VdbeAddOp3(v, OP_Column, iIdxCur,
-						  part->fieldno, regPrev + i);
-			}
-			sqlite3VdbeResolveLabel(v, endDistinctTest);
-			sqlite3DbFree(db, aGotoChng);
+		/*
+		 *  chng_addr_0:
+		 *   regPrev(0) = idx(0)
+		 *  chng_addr_1:
+		 *   regPrev(1) = idx(1)
+		 *  ...
+		 */
+		sqlite3VdbeJumpHere(v, addrNextRow - 1);
+		part = idx->def->key_def->parts;
+		for (i = 0; i < part_count; ++i, ++part) {
+			sqlite3VdbeJumpHere(v, aGotoChng[i]);
+			sqlite3VdbeAddOp3(v, OP_Column, iIdxCur, part->fieldno,
+					  regPrev + i);
  		}
+		sqlite3VdbeResolveLabel(v, endDistinctTest);
+		sqlite3DbFree(db, aGotoChng);
  
  		/*
  		 *  chng_addr_N:
@@ -989,14 +988,14 @@ analyzeOneTable(Parse * pParse,	/* Parser context */
  		 *   if !eof(csr) goto next_row;
  		 */
  		assert(regKey == (regStat4 + 2));
-		struct index *pPk = sql_table_primary_key(pTab);
-		int pk_part_count = pPk->def->key_def->part_count;
+		struct index *pk = sql_table_primary_key(pTab);
+		int pk_part_count = pk->def->key_def->part_count;
  		/* Allocate memory for array. */
  		pParse->nMem = MAX(pParse->nMem,
  				   regPrev + part_count + pk_part_count);
  		int regKeyStat = regPrev + part_count;
  		for (i = 0; i < pk_part_count; i++) {
-			uint32_t k = pPk->def->key_def->parts[i].fieldno;
+			uint32_t k = pk->def->key_def->parts[i].fieldno;
  			assert(k < pTab->def->field_count);
  			sqlite3VdbeAddOp3(v, OP_Column, iIdxCur, k,
  					  regKeyStat + i);
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 79dc46592..534afc154 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -273,8 +273,8 @@ table_delete(struct sqlite3 *db, struct Table *tab)
  		free(idx->def);
  		free(idx);
  	}
-	free(tab->space);
  	free(tab->space->index);
+	free(tab->space);
  skip_index_delete:
  	assert(tab->def != NULL);
  	/* Do not delete table->def allocated on region. */
@@ -1214,8 +1214,8 @@ vdbe_emit_create_index(struct Parse *parse, struct space_def *def,
   * @param idx_name Name of index to be created.
   * @param space_id Space id (or register containing it)
   *                 which index belongs to.
- * @param iIndexId Id of index (or register containing it)
- *                 to be created.
+ * @param iid Id of index (or register containing it) to be
+ *        created.
   * @param sql_stmt String containing 'CREATE INDEX ...' statement.
   *                 NULL for UNIQUE and PK constraints.
   */
@@ -1229,7 +1229,7 @@ vdbe_emit_index_schema_record(struct Parse *parse, const char *idx_name,
  
  	sqlite3VdbeAddOp4(v, OP_String8, 0, entry_reg, 0,
  			  sqlite3DbStrDup(parse->db, idx_name), P4_DYNAMIC);
-	if (parse->pNewTable) {
+	if (parse->pNewTable != NULL) {
  		/*
  		 * A new table is being created, hence space_id
  		 * is a register, but index id is literal.
@@ -1327,11 +1327,12 @@ parseTableSchemaRecord(Parse * pParse, int iSpaceId, char *zStmt)
  	sqlite3VdbeAddOp2(v, OP_Integer, 0, iTop + 2);
  	sqlite3VdbeAddOp4(v, OP_String8, 0, iTop + 3, 0, zStmt, P4_DYNAMIC);
  
-	if (!p->def->opts.is_view)
+	if (!p->def->opts.is_view) {
  		for (uint32_t i = 1; i < p->space->index_count; ++i) {
  			const char *idx_name = p->space->index[i]->def->name;
  			vdbe_emit_index_schema_record(pParse, idx_name,
  						      iSpaceId, i, NULL);
+		}
  	}
  
  	sqlite3VdbeAddParseSchema2Op(v, iTop, pParse->nMem - iTop + 1);
@@ -2464,7 +2465,7 @@ table_add_index(struct Table *tab, struct index *index)
  	struct index **idx = (struct index **) realloc(tab->space->index,
  						       indexes_sz);
  	if (idx == NULL) {
-		diag_set(OutOfMemory, indexes_sz, "malloc", "idx");
+		diag_set(OutOfMemory, indexes_sz, "realloc", "idx");
  		return;
  	}
  	tab->space->index = idx;
@@ -3010,7 +3011,7 @@ sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list,
  	 * data-dictionaries will be completely merged.
  	 */
  	struct Table *tab = sqlite3HashFind(&db->pSchema->tblHash, table_name);
-	sqlite3VdbeAddOp3(v, OP_DropIndex, index->def->iid, 0, 0);
+	sqlite3VdbeAddOp1(v, OP_DropIndex, index->def->iid);
  	sqlite3VdbeAppendP4(v, tab->space, P4_SPACEPTR);
  
   exit_drop_index:
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 9220d34e1..ce704c0a2 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -1113,22 +1113,9 @@ sql_index_is_xfer_compatible(const struct index_def *dest,
  {
  	assert(dest != NULL && src != NULL);
  	assert(dest->space_id != src->space_id);
-	uint32_t dest_idx_part_count = dest->key_def->part_count;
-	uint32_t src_idx_part_count = src->key_def->part_count;
-	if (dest_idx_part_count != src_idx_part_count)
-		return false;
-	struct key_part *src_part = src->key_def->parts;
-	struct key_part *dest_part = dest->key_def->parts;
-	for (uint32_t i = 0; i < src_idx_part_count;
-	     ++i, ++src_part, ++dest_part) {
-		if (src_part->fieldno != dest_part->fieldno)
-			return false;
-		if (src_part->sort_order != dest_part->sort_order)
-			return false;
-		if (src_part->coll != dest_part->coll)
-			return false;
-	}
-	return true;
+	return key_part_cmp(src->key_def->parts, src->key_def->part_count,
+			    dest->key_def->parts,
+			    dest->key_def->part_count) == 0;
  }
  
  /*
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 19a5e7dd9..2bc6a98f6 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3083,7 +3083,6 @@ struct Walker {
  		SrcList *pSrcList;	/* FROM clause */
  		struct SrcCount *pSrcCount;	/* Counting column references */
  		int *aiCol;	/* array of column indexes */
-		struct IdxCover *pIdxCover;	/* Check for index coverage */
  		/** Space definition. */
  		struct space_def *space_def;
  	} u;
@@ -3682,7 +3681,6 @@ int sqlite3ExprCodeExprList(Parse *, ExprList *, int, int, u8);
  #define SQLITE_ECEL_OMITREF  0x08	/* Omit if ExprList.u.x.iOrderByCol */
  void sqlite3ExprIfTrue(Parse *, Expr *, int, int);
  void sqlite3ExprIfFalse(Parse *, Expr *, int, int);
-void sqlite3ExprIfFalseDup(Parse *, Expr *, int, int);
  #define LOCATE_VIEW    0x01
  #define LOCATE_NOERR   0x02
  Table *sqlite3LocateTable(Parse *, u32 flags, const char *);
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index 6ee83a5de..3ba01198d 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -229,10 +229,10 @@ void sqlite3VdbeAppendP4(Vdbe *, void *pP4, int p4type);
   * Set the P4 on the most recently added opcode to the key_def for the
   * index given.
   * @param Parse context, for error reporting.
- * @param idx_def Definition of index to get key_def from.
+ * @param key_def Definition of a key to set.
   */
  void
-sql_vdbe_set_p4_key_def(struct Parse *parse, struct index_def *idx_def);
+sql_vdbe_set_p4_key_def(struct Parse *parse, struct key_def *key_def);
  
  VdbeOp *sqlite3VdbeGetOp(Vdbe *, int);
  int sqlite3VdbeMakeLabel(Vdbe *);
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 12c2edffe..dda030234 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -1055,16 +1055,16 @@ sqlite3VdbeAppendP4(Vdbe * p, void *pP4, int n)
  }
  
  void
-sql_vdbe_set_p4_key_def(struct Parse *parse, struct index_def *idx_def)
+sql_vdbe_set_p4_key_def(struct Parse *parse, struct key_def *key_def)
  {
  	struct Vdbe *v = parse->pVdbe;
  	assert(v != NULL);
-	assert(idx_def != NULL);
-	struct key_def *def = key_def_dup(idx_def->key_def);
-	if (def == NULL)
+	assert(key_def != NULL);
+	key_def = key_def_dup(key_def);
+	if (key_def == NULL)
  		sqlite3OomFault(parse->db);
  	else
-		sqlite3VdbeAppendP4(v, def, P4_KEYDEF);
+		sqlite3VdbeAppendP4(v, key_def, P4_KEYDEF);
  }
  
  #ifdef SQLITE_ENABLE_EXPLAIN_COMMENTS
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index d8a2c1a79..1b9ba436c 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -617,19 +617,18 @@ isDistinctRedundant(Parse * pParse,		/* Parsing context */
  	 *      contain a "col=X" term are subject to a NOT NULL constraint.
  	 */
  	for (uint32_t j = 0; j < pTab->space->index_count; ++j) {
-		struct index *idx = pTab->space->index[j];
-		if (!idx->def->opts.is_unique)
+		struct index_def *def = pTab->space->index[j]->def;
+		if (!def->opts.is_unique)
  			continue;
-		uint32_t col_count = idx->def->key_def->part_count;
+		uint32_t col_count = def->key_def->part_count;
  		uint32_t i;
  		for (i = 0; i < col_count; i++) {
-			if (0 ==
-			    sqlite3WhereFindTerm(pWC, iBase, i, ~(Bitmask) 0,
-						 WO_EQ, idx->def)) {
-				if (findIndexCol
-				    (pParse, pDistinct, iBase, idx->def, i) < 0)
+			if (sqlite3WhereFindTerm(pWC, iBase, i, ~(Bitmask) 0,
+						 WO_EQ, def) == 0) {
+				if (findIndexCol(pParse, pDistinct, iBase, def,
+						 i) < 0)
  					break;
-				uint32_t x = idx->def->key_def->parts[i].fieldno;
+				uint32_t x = def->key_def->parts[i].fieldno;
  				if (pTab->def->fields[x].is_nullable)
  					break;
  			}
@@ -638,7 +637,7 @@ isDistinctRedundant(Parse * pParse,		/* Parsing context */
  		 * This index implies that the DISTINCT
  		 * qualifier is redundant.
  		 */
-		if (i == idx->def->key_def->part_count)
+		if (i == col_count)
  			return 1;
  	}
  
@@ -858,7 +857,7 @@ constructAutomaticIndex(Parse * pParse,			/* The parsing context */
  	assert(pLevel->iIdxCur >= 0);
  	pLevel->iIdxCur = pParse->nTab++;
  	sqlite3VdbeAddOp2(v, OP_OpenAutoindex, pLevel->iIdxCur, nKeyCol + 1);
-	sql_vdbe_set_p4_key_def(pParse, pIdx);
+	sql_vdbe_set_p4_key_def(pParse, pIdx->key_def);
  	VdbeComment((v, "for %s", pTable->def->name));
  
  	/* Fill the automatic index with content */
@@ -2827,6 +2826,7 @@ whereLoopAddBtree(WhereLoopBuilder * pBuilder,	/* WHERE clause information */
  		memset(&fake_index, 0, sizeof(fake_index));
  		struct key_def *key_def = key_def_new(1);
  		if (key_def == NULL) {
+tnt_error:
  			pWInfo->pParse->nErr++;
  			pWInfo->pParse->rc = SQL_TARANTOOL_ERROR;
  			return SQL_TARANTOOL_ERROR;
@@ -2843,18 +2843,18 @@ whereLoopAddBtree(WhereLoopBuilder * pBuilder,	/* WHERE clause information */
  					   sizeof("fake_autoindex") - 1,
  					   TREE, &opts, key_def, NULL);
  		key_def_delete(key_def);
-		if (fake_index == NULL) {
-			pWInfo->pParse->nErr++;
-			pWInfo->pParse->rc = SQL_TARANTOOL_ERROR;
-			return SQL_TARANTOOL_ERROR;
-		}
+		if (fake_index == NULL)
+			goto tnt_error;
  		/* Special marker for  non-existent index. */
  		fake_index->iid = UINT32_MAX;
+		int size = sizeof(struct index_stat) + sizeof(log_est_t) * 2;
  
-		struct index_stat *stat =
-			(struct index_stat *) malloc(sizeof(struct index_stat));
-		stat->tuple_log_est =
-			(log_est_t *) malloc(sizeof(log_est_t) * 2);
+		struct index_stat *stat = (struct index_stat *) malloc(size);
+		if (stat == NULL) {
+			diag_set(OutOfMemory, size, "malloc", "stat");
+			goto tnt_error;
+		}
+		stat->tuple_log_est = (log_est_t *) ((char *) (stat + 1));
  		stat->tuple_log_est[0] = sql_space_tuple_log_count(pTab);
  		stat->tuple_log_est[1] = 0;
  		fake_index->opts.stat = stat;
@@ -2917,7 +2917,12 @@ whereLoopAddBtree(WhereLoopBuilder * pBuilder,	/* WHERE clause information */
  		}
  	}
  #endif				/* SQLITE_OMIT_AUTOMATIC_INDEX */
-	uint32_t idx_count = fake_index == NULL ? pTab->space->index_count : 1;
+	/*
+	 * If there was an INDEXED BY clause, then only that one
+	 * index is considered.
+	 */
+	uint32_t idx_count = fake_index == NULL || pSrc->pIBIndex != NULL ?
+			     pTab->space->index_count : 1;
  	for (uint32_t i = 0; i < idx_count; iSortIdx++, i++) {
  		if (i > 0)
  			probe = pTab->space->index[i]->def;
@@ -2971,20 +2976,9 @@ whereLoopAddBtree(WhereLoopBuilder * pBuilder,	/* WHERE clause information */
  		sqlite3Stat4ProbeFree(pBuilder->pRec);
  		pBuilder->nRecValid = 0;
  		pBuilder->pRec = 0;
-
-		/* If there was an INDEXED BY clause, then only that one index is
-		 * considered.
-		 */
-		if (pSrc->pIBIndex != NULL)
-			break;
-		if (fake_index != NULL)
-			break;
-
  	}
-	if (fake_index != NULL) {
-		free(fake_index->opts.stat->tuple_log_est);
+	if (fake_index != NULL)
  		index_def_delete(fake_index);
-	}
  	return rc;
  }
  
@@ -3209,7 +3203,7 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo,	/* The WHERE clause */
  	WhereLoop *pLoop = 0;	/* Current WhereLoop being processed. */
  	WhereTerm *pTerm;	/* A single term of the WHERE clause */
  	Expr *pOBExpr;		/* An expression from the ORDER BY clause */
-	struct index_def *pIndex;
+	struct index_def *idx_def;
  	sqlite3 *db = pWInfo->pParse->db;	/* Database connection */
  	Bitmask obSat = 0;	/* Mask of ORDER BY terms satisfied so far */
  	Bitmask obDone;		/* Mask of all ORDER BY terms */
@@ -3313,14 +3307,14 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo,	/* The WHERE clause */
  
  		if ((pLoop->wsFlags & WHERE_ONEROW) == 0) {
  			if (pLoop->wsFlags & WHERE_IPK) {
-				pIndex = NULL;
+				idx_def = NULL;
  				nColumn = 1;
-			} else if ((pIndex = pLoop->index_def) == NULL ||
-				   index_is_unordered(pIndex)) {
+			} else if ((idx_def = pLoop->index_def) == NULL ||
+				   index_is_unordered(idx_def)) {
  				return 0;
  			} else {
-				nColumn = pIndex->key_def->part_count;
-				isOrderDistinct = pIndex->opts.is_unique;
+				nColumn = idx_def->key_def->part_count;
+				isOrderDistinct = idx_def->opts.is_unique;
  			}
  
  			/* Loop through all columns of the index and deal with the ones
@@ -3380,8 +3374,8 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo,	/* The WHERE clause */
  				/* Get the column number in the table (iColumn) and sort order
  				 * (revIdx) for the j-th column of the index.
  				 */
-				if (pIndex != NULL) {
-					struct key_def *def = pIndex->key_def;
+				if (idx_def != NULL) {
+					struct key_def *def = idx_def->key_def;
  					iColumn = def->parts[j].fieldno;
  					revIdx = def->parts[j].sort_order;
  				} else {
@@ -3393,9 +3387,9 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo,	/* The WHERE clause */
  				 * WhereLoop is not well-ordered
  				 */
  				if (isOrderDistinct && iColumn >= 0 &&
-				    j >= pLoop->nEq && pIndex != NULL) {
+				    j >= pLoop->nEq && idx_def != NULL) {
  					struct space *space =
-						space_by_id(pIndex->space_id);
+						space_by_id(idx_def->space_id);
  					assert(space != NULL);
  					if (space->def->fields[iColumn].is_nullable)
  						isOrderDistinct = 0;
@@ -3432,7 +3426,7 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo,	/* The WHERE clause */
  								      pOrderBy->a[i].pExpr,
  								      &is_found, &id);
  						struct coll *idx_coll =
-							pIndex->key_def->parts[j].coll;
+							idx_def->key_def->parts[j].coll;
  						if (is_found &&
  						    coll != idx_coll)
  							continue;
@@ -4630,9 +4624,7 @@ sqlite3WhereBegin(Parse * pParse,	/* The parser context */
  			 */
  			if (idx_def == NULL)
  				continue;
-			bool is_primary = (idx_def != NULL &&
-					   idx_def->iid == 0);
-			if (is_primary
+			if (idx_def->iid == 0
  			    && (wctrlFlags & WHERE_OR_SUBCLAUSE) != 0) {
  				/* This is one term of an OR-optimization using
  				 * the PRIMARY KEY.  No need for a separate index
@@ -4640,8 +4632,7 @@ sqlite3WhereBegin(Parse * pParse,	/* The parser context */
  				iIndexCur = pLevel->iTabCur;
  				op = 0;
  			} else if (pWInfo->eOnePass != ONEPASS_OFF) {
-				if (idx_def != NULL &&
-				    pTabItem->pTab->space->index_count != 0) {
+				if (pTabItem->pTab->space->index_count != 0) {
  					uint32_t iid = 0;
  					struct index *pJ = pTabItem->pTab->space->index[iid];
  					iIndexCur = iAuxArg;
@@ -4675,19 +4666,10 @@ sqlite3WhereBegin(Parse * pParse,	/* The parser context */
  			pLevel->iIdxCur = iIndexCur;
  			assert(iIndexCur >= 0);
  			if (op) {
-				if (idx_def != NULL) {
-					uint32_t space_id =
-						idx_def->space_id;
-					struct space *space =
-						space_by_id(space_id);
-					vdbe_emit_open_cursor(pParse, iIndexCur,
-							      idx_def->iid,
-							      space);
-				} else {
-					vdbe_emit_open_cursor(pParse, iIndexCur,
-							      idx_def->iid,
-							      space);
-				}
+				uint32_t space_id = idx_def->space_id;
+				struct space *space = space_by_id(space_id);
+				vdbe_emit_open_cursor(pParse, iIndexCur,
+						      idx_def->iid, space);
  				if ((pLoop->wsFlags & WHERE_CONSTRAINT) != 0
  				    && (pLoop->
  					wsFlags & (WHERE_COLUMN_RANGE |
@@ -4696,8 +4678,7 @@ sqlite3WhereBegin(Parse * pParse,	/* The parser context */
  					wctrlFlags & WHERE_ORDERBY_MIN) == 0) {
  					sqlite3VdbeChangeP5(v, OPFLAG_SEEKEQ);	/* Hint to COMDB2 */
  				}
-				if (idx_def != NULL)
-					VdbeComment((v, "%s", idx_def->name));
+				VdbeComment((v, "%s", idx_def->name));
  #ifdef SQLITE_ENABLE_COLUMN_USED_MASK
  				{
  					u64 colUsed = 0;
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 628c6f9ad..5d8663a92 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -219,7 +219,7 @@ sqlite3WhereExplainOneScan(Parse * pParse,	/* Parse context */
  
  			assert(!(flags & WHERE_AUTO_INDEX)
  			       || (flags & WHERE_IDX_ONLY));
-			if (idx_def != NULL && idx_def->iid == 0) {
+			if (idx_def->iid == 0) {
  				if (isSearch) {
  					zFmt = "PRIMARY KEY";
  				}
@@ -234,10 +234,7 @@ sqlite3WhereExplainOneScan(Parse * pParse,	/* Parse context */
  			}
  			if (zFmt) {
  				sqlite3StrAccumAppend(&str, " USING ", 7);
-				if (idx_def != NULL)
-					sqlite3XPrintf(&str, zFmt, idx_def->name);
-				else
-					sqlite3XPrintf(&str, zFmt, "EPHEMERAL INDEX");
+				sqlite3XPrintf(&str, zFmt, idx_def->name);
  				explainIndexRange(&str, pLoop);
  			}
  		} else if ((flags & WHERE_IPK) != 0
@@ -477,7 +474,7 @@ codeEqualityTerm(Parse * pParse,	/* The parsing context */
  		int nEq = 0;
  		int *aiMap = 0;
  
-		if (pLoop->index_def != 0 &&
+		if (pLoop->index_def != NULL &&
  		    pLoop->index_def->key_def->parts[iEq].sort_order) {
  			testcase(iEq == 0);
  			testcase(bRev);
@@ -1187,7 +1184,7 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
  			int limit = pRangeStart == NULL ? nEq : nEq + 1;
  			for (int i = 0; i < limit; i++) {
  				if (idx_def->key_def->parts[i].fieldno ==
-				     idx_pk->key_def->parts[0].fieldno) {
+				    idx_pk->key_def->parts[0].fieldno) {
  					/* Here: we know for sure that table has INTEGER
  					   PRIMARY KEY, single column, and Index we're
  					   trying to use for scan contains this column. */
@@ -1295,11 +1292,9 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
  		if (omitTable) {
  			/* pIdx is a covering index.  No need to access the main table. */
  		}  else if (iCur != iIdxCur) {
-			struct index *pPk = space->index_map[0];
-			int pk_part_count = pPk->def->key_def->part_count;
  			int iKeyReg = sqlite3GetTempRange(pParse, pk_part_count);
-			for (j = 0; j < pk_part_count; j++) {
-				k = pPk->def->key_def->parts[j].fieldno;
+			for (j = 0; j < (int) pk_part_count; j++) {
+				k = idx_pk->key_def->parts[j].fieldno;
  				sqlite3VdbeAddOp3(v, OP_Column, iIdxCur, k,
  						  iKeyReg + j);
  			}
@@ -1344,7 +1339,7 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
  		 */
  		WhereClause *pOrWc;	/* The OR-clause broken out into subterms */
  		SrcList *pOrTab;	/* Shortened table list or OR-clause generation */
-		struct index_def *pCov = 0;	/* Potential covering index (or NULL) */
+		struct index_def *cov = NULL;	/* Potential covering index (or NULL) */
  		int iCovCur = pParse->nTab++;	/* Cursor used for index scans (if any) */
  
  		int regReturn = ++pParse->nMem;	/* Register used with OP_Gosub */
@@ -1357,6 +1352,9 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
  		u16 wctrlFlags;	/* Flags for sub-WHERE clause */
  		Expr *pAndExpr = 0;	/* An ".. AND (...)" expression */
  		Table *pTab = pTabItem->pTab;
+		struct key_def *pk_key_def =
+			sql_table_primary_key(pTab)->def->key_def;
+		uint32_t pk_part_count = pk_key_def->part_count;
  
  		pTerm = pLoop->aLTerm[0];
  		assert(pTerm != 0);
@@ -1403,12 +1401,10 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
  		 * called on an uninitialized cursor.
  		 */
  		if ((pWInfo->wctrlFlags & WHERE_DUPLICATES_OK) == 0) {
-			struct index *pPk = sql_table_primary_key(pTab);
-			int pk_part_count = pPk->def->key_def->part_count;
  			regRowset = pParse->nTab++;
  			sqlite3VdbeAddOp2(v, OP_OpenTEphemeral,
  					  regRowset, pk_part_count);
-			sql_vdbe_set_p4_key_def(pParse, pPk->def);
+			sql_vdbe_set_p4_key_def(pParse, pk_key_def);
  			regPk = ++pParse->nMem;
  		}
  		iRetInit = sqlite3VdbeAddOp2(v, OP_Integer, 0, regReturn);
@@ -1507,19 +1503,15 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
  						int r;
  						int iSet =
  						    ((ii == pOrWc->nTerm - 1) ? -1 : ii);
-						struct index *pPk =
-							sql_table_primary_key(pTab);
-						struct key_def *def =
-							pPk->def->key_def;
  
  						/* Read the PK into an array of temp registers. */
  						r = sqlite3GetTempRange(pParse,
-									def->part_count);
+									pk_part_count);
  						for (uint32_t iPk = 0;
-						     iPk < def->part_count;
+						     iPk < pk_part_count;
  						     iPk++) {
  							uint32_t fieldno =
-								def->parts[iPk].
+								pk_key_def->parts[iPk].
  								fieldno;
  							sqlite3ExprCodeGetColumnToReg
  								(pParse,
@@ -1546,20 +1538,20 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
  								(v, OP_Found,
  								 regRowset, 0,
  								 r,
-								 def->part_count);
+								 pk_part_count);
  							VdbeCoverage(v);
  						}
  						if (iSet >= 0) {
  							sqlite3VdbeAddOp3
  								(v, OP_MakeRecord,
-								 r, def->part_count, regPk);
+								 r, pk_part_count, regPk);
  							sqlite3VdbeAddOp2
  								(v, OP_IdxInsert,
  								 regRowset, regPk);
  						}
  
  						/* Release the array of temp registers */
-						sqlite3ReleaseTempRange(pParse, r, def->part_count);
+						sqlite3ReleaseTempRange(pParse, r, pk_part_count);
  					}
  
  					/* Invoke the main loop body as a subroutine */
@@ -1588,21 +1580,21 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
  					 * If the call to sqlite3WhereBegin() above resulted in a scan that
  					 * uses an index, and this is either the first OR-connected term
  					 * processed or the index is the same as that used by all previous
-					 * terms, set pCov to the candidate covering index. Otherwise, set
-					 * pCov to NULL to indicate that no candidate covering index will
+					 * terms, set cov to the candidate covering index. Otherwise, set
+					 * cov to NULL to indicate that no candidate covering index will
  					 * be available.
  					 */
  					pSubLoop = pSubWInfo->a[0].pWLoop;
  					assert((pSubLoop->wsFlags & WHERE_AUTO_INDEX) == 0);
  					if ((pSubLoop->wsFlags & WHERE_INDEXED) != 0
-					    && (ii == 0 || (pCov != NULL &&
-						pSubLoop->index_def->iid == pCov->iid))
+					    && (ii == 0 || (cov != NULL &&
+						pSubLoop->index_def->iid == cov->iid))
  					    && (pSubLoop->index_def->iid != 0)) {
  						assert(pSubWInfo->a[0].
  						       iIdxCur == iCovCur);
-						pCov = pSubLoop->index_def;
+						cov = pSubLoop->index_def;
  					} else {
-						pCov = 0;
+						cov = 0;
  					}
  
  					/* Finish the loop through table entries that match term pOrTerm. */
@@ -1610,8 +1602,8 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
  				}
  			}
  		}
-		pLevel->u.pCovidx = pCov;
-		if (pCov)
+		pLevel->u.pCovidx = cov;
+		if (cov)
  			pLevel->iIdxCur = iCovCur;
  		if (pAndExpr) {
  			pAndExpr->pLeft = 0;