[tarantool-patches] Re: [PATCH 3/3] sql: implement point where for DELETE stmts

["n.pettik" <korablev@tarantool.org>]

For some reason this patch got to the trunk without review fixes.
Raw version of this patch contains several bugs, so they led to test fails
(sql-tap/analyze6.test.lua and few sql-tap/tkt*).

I suggest my own fixes. The whole patch is at the end of letter.

Branch: https://github.com/tarantool/tarantool/commits/np/gh-3463-review-fixed-for-point-delete
Issue: https://github.com/tarantool/tarantool/issues/3463

> 
>>  }
>>    struct ExprList *
>> @@ -1221,6 +1216,22 @@ emit_open_cursor(Parse *parse_context, int cursor, int entity_id)
>>  				 space_ptr_reg);
>>  }
>>  +int
>> +sql_emit_open_cursor(struct Parse *parse, int cursor, int index_id, struct space *space)
> 
> 2. Few lines above I see emit_open_cursor. Please, try to remove one of them.
> At the worst case you can just inline emit_open_cursor in the single place of
> its usage, and rename sql_emit_open_cursor to emit_open_cursor.

Ok, I just replaced all calls of emit_open_cursor() with sql_emit_open_cursor().
The only difference is in doing space lookup outside emit_open_cursor().

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 552578048..8c83a797e 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1181,35 +1181,17 @@ space_checks_expr_list(uint32_t space_id)
 }
 
 int
-emit_open_cursor(struct Parse *parse_context, int cursor, int entity_id)
+sql_emit_open_cursor(struct Parse *parse_context, int cursor, int index_id,
+                    struct space *space)
 {
-       assert(entity_id > 0);
-       struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(entity_id));
        assert(space != NULL);
        struct Vdbe *vdbe = parse_context->pVdbe;
        int space_ptr_reg = ++parse_context->nMem;
        sqlite3VdbeAddOp4(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
                          P4_SPACEPTR);
-       return sqlite3VdbeAddOp3(vdbe, OP_OpenWrite, cursor, entity_id,
+       return sqlite3VdbeAddOp3(vdbe, OP_OpenWrite, cursor, index_id,
                                 space_ptr_reg);
 }
-
-int
-sql_emit_open_cursor(struct Parse *parse, int cursor, int index_id, struct space *space)
-{
-       assert(space != NULL);
-       Vdbe *vdbe = parse->pVdbe;
-       int space_ptr_reg = ++parse->nMem;
-       sqlite3VdbeAddOp4(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
-                         P4_SPACEPTR);
-       struct key_def *def = key_def_dup(space->index[index_id]->def->key_def);
-       if (def == NULL)
-               return 0;
-       return sqlite3VdbeAddOp4(vdbe, OP_OpenWrite, cursor, index_id,
-                                space_ptr_reg,
-                                (char*)def,
-                                P4_KEYDEF);
-}

> 
>> +{
>> +	assert(space != NULL);
>> +	Vdbe *vdbe = parse->pVdbe;
>> +	int space_ptr_reg = ++parse->nMem;
>> +	sqlite3VdbeAddOp4(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
>> +			  P4_SPACEPTR);
>> +	struct key_def *def = key_def_dup(space->index[index_id]->def->key_def);
>> +	if (def == NULL)
>> +		return 0;
>> +	return sqlite3VdbeAddOp4(vdbe, OP_OpenWrite, cursor, index_id,
>> +				 space_ptr_reg,
>> +				 (char*)def,
>> +				 P4_KEYDEF);
> 
> 3. Looks like the arguments can fit in 2 lines instead of 4.

Fixed. See code above.

> 
>> +}
>> diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
>> index 2b59130..de4e0c1 100644
>> --- a/src/box/sql/delete.c
>> +++ b/src/box/sql/delete.c
>> @@ -184,7 +184,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
>>  		memset(&nc, 0, sizeof(nc));
>>  		nc.pParse = parse;
>>  		if (tab_list->a[0].pTab == NULL) {
>> -			struct Table *t = malloc(sizeof(struct Table));
>> +			struct Table *t = calloc(sizeof(struct Table), 1);
> 
> 4. It must the part of the previous patch.

Refactored this way:

+++ b/src/box/sql/delete.c
@@ -88,7 +88,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
         * instead of just a Table* parameter.
         */
        struct Table *table = NULL;
-       struct space *space;
+       struct space *space = NULL;
        uint32_t space_id;
        /* Figure out if we have any triggers and if the table
         * being deleted from is a view.
@@ -104,17 +104,28 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
                                                strlen(tab_name));
                if (space_id == BOX_ID_NIL)
                        goto delete_from_cleanup;
+               /*
+                * In this case space has been created via Lua
+                * facilities, so there is no corresponding entry
+                * in table hash. Thus, lets create simple
+                * wrapper around space_def to support interface.
+                */
+               space = space_by_id(space_id);
+               tab_list->a[0].pTab = sql_table_construct_from_space(space);
+               if (tab_list->a[0].pTab == NULL)
+                       goto delete_from_cleanup;
        } else {
                table = sql_list_lookup_table(parse, tab_list);
                if (table == NULL)
                        goto delete_from_cleanup;
                space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
+               space = space_by_id(space_id);
+               assert(space != NULL);

@@ -183,16 +194,6 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
                struct NameContext nc;
                memset(&nc, 0, sizeof(nc));
                nc.pParse = parse;
-               if (tab_list->a[0].pTab == NULL) {
-                       struct Table *t = calloc(sizeof(struct Table), 1);
-                       if (t == NULL) {
-                               sqlite3OomFault(parse->db);
-                               goto delete_from_cleanup;
-                       }
-                       assert(space != NULL);
-                       t->def = space->def;
-                       tab_list->a[0].pTab = t;
-               }

+++ b/src/box/sql/resolve.c
@@ -39,6 +39,9 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "box/box.h"
+#include "box/schema.h"
+
 /*
  * Walk the expression tree pExpr and increase the aggregate function
  * depth (the Expr.op2 field) by N on every TK_AGG_FUNCTION node.
@@ -1432,6 +1435,21 @@ resolveSelectStep(Walker * pWalker, Select * p)
        return WRC_Prune;
 }
 
+struct Table *
+sql_table_construct_from_space(const struct space *space)
+{
+       assert(space != NULL);
+       struct Table *table = calloc(1, sizeof(*table));
+       if (table == NULL) {
+               diag_set(OutOfMemory, sizeof(table), "calloc", "table");
+               return NULL;
+       }
+       table->def = space_def_dup(space->def);
+       if (table->def == NULL)
+               return NULL;
+       return table;
+}
+

+/**
+ * Create wrapper around space_def of the given space.
+ * This routine is required to support.
+ * Both table and space def are allocated on heap.
+ *
+ * @param space Space to be prototype.
+ * @retval New instance of struct Table allocated on malloc,
+ *         or NULL in case of OOM.
+ */
+struct Table *
+sql_table_construct_from_space(const struct space *space);

> 
>>  			if (t == NULL) {
>>  				sqlite3OomFault(parse->db);
>>  				goto delete_from_cleanup;
>> @@ -266,7 +266,12 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
>>  		/* Extract the primary key for the current row */
>>  		if (!is_view) {
>>  			for (int i = 0; i < pk_len; i++) {
>> -				sqlite3ExprCodeGetColumnOfTable(v, table->def,
>> +				struct space_def *def;
>> +				if (table != NULL)
>> +					def = table->def;
>> +				else
>> +					def = space->def;
> 
> 5. Why not always space->def?

Idk, indeed lets always use space->def:

@@ -266,11 +267,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
                /* Extract the primary key for the current row */
                if (!is_view) {
                        for (int i = 0; i < pk_len; i++) {
-                               struct space_def *def;
-                               if (table != NULL)
-                                       def = table->def;
-                               else
-                                       def = space->def;
+                               struct space_def *def = space->def;

> 
>> +				sqlite3ExprCodeGetColumnOfTable(v, def,
>>  								tab_cursor,
>>  								pk_def->parts[i].fieldno,
>>  								reg_pk + i);
>> @@ -339,8 +344,18 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
>>  			int space_ptr_reg = ++parse->nMem;
>>  			sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
>>  					  (void *)space, P4_SPACEPTR);
>> +
>> +			int tnum;
>> +			if (table != NULL) {
>> +				tnum = table->tnum;
>> +			}
>> +			else {
>> +				/* index id is 0 for PK.  */
>> +				tnum = SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(space->def->id,
>> +									      0);
>> +			}
> 
> 6. Why not always the second version?

Idk as well, lets always use conversion from space->def->id.

@@ -342,15 +339,9 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
                        sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
                                          (void *)space, P4_SPACEPTR);
 
-                       int tnum;
-                       if (table != NULL) {
-                               tnum = table->tnum;
-                       }
-                       else {
-                               /* index id is 0 for PK.  */
-                               tnum = SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(space->def->id,
-                                                                             0);
-                       }
+                       int tnum =
+                               SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(space->def->id,
+                                                                      0);

> 
>>  			sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor,
>> -					  table->tnum, space_ptr_reg);
>> +					  tnum, space_ptr_reg);
>>  			struct key_def *def = key_def_dup(pk_def);
>>  			if (def == NULL) {
>>  				sqlite3OomFault(parse->db);
>> @@ -505,7 +521,7 @@ sql_generate_row_delete(struct Parse *parse, struct Table *table,
>>  	 * of the DELETE statement is to fire the INSTEAD OF
>>  	 * triggers).
>>  	 */
>> -	if (table->pSelect == NULL) {
>> +	if (table == NULL || table->pSelect == NULL) {
> 
> 7. Is the same as space->def->opts.is_view.

Ok:

@@ -518,7 +509,7 @@ sql_generate_row_delete(struct Parse *parse, struct Table *table,
         * of the DELETE statement is to fire the INSTEAD OF
         * triggers).
         */
-       if (table == NULL || table->pSelect == NULL) {
+       if (table == NULL || !table->def->opts.is_view) {

> 
>>  		uint8_t p5 = 0;
>>  		sqlite3VdbeAddOp2(v, OP_Delete, cursor,
>>  				  (need_update_count ? OPFLAG_NCHANGE : 0));
>> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
>> index 3dbf855..8d0ab3b 100644
>> --- a/src/box/sql/insert.c
>> +++ b/src/box/sql/insert.c
>> @@ -108,6 +108,39 @@ sqlite3IndexAffinityStr(sqlite3 * db, Index * pIdx)
>>  	return pIdx->zColAff;
>>  }
>>  +char *
>> +sql_index_affinity_str(struct sqlite3 * db, struct index_def *def)
>> +{
>> +	char *aff;
>> +	/* The first time a column affinity string for a particular index is
>> +	 * required, it is allocated and populated here. It is then stored as
>> +	 * a member of the Index structure for subsequent use.
> 
> 8. No, it is not populated.

Deleted the whole comment - it doesn’t seem to be relevant.
And slightly refactored function:
 
@@ -107,32 +107,22 @@ sqlite3IndexAffinityStr(sqlite3 *db, Index *index)
 char *
 sql_index_affinity_str(struct sqlite3 * db, struct index_def *def)
 {
-       char *aff;
-       /* The first time a column affinity string for a particular index is
-        * required, it is allocated and populated here. It is then stored as
-        * a member of the Index structure for subsequent use.
-        *
-        * The column affinity string will eventually be deleted by
-        * sqliteDeleteIndex() when the Index structure itself is cleaned
-        * up.
-        */
-       int nColumn = def->key_def->part_count;
-       aff = (char *)sqlite3DbMallocRaw(0, nColumn + 1);
+       uint32_t column_count = def->key_def->part_count;
+       char *aff = (char *)sqlite3DbMallocRaw(0, column_count + 1);
        if (aff == NULL) {
                sqlite3OomFault(db);
-               return 0;
+               return NULL;
        }
-       int i;
-       struct space *space = space_cache_find(def->space_id);
+       struct space *space = space_by_id(def->space_id);
        assert(space != NULL);
 
-       for (i = 0; i < nColumn; i++) {
+       for (uint32_t i = 0; i < column_count; i++) {
                uint32_t x = def->key_def->parts[i].fieldno;
                aff[i] = space->def->fields[x].affinity;
                if (aff[i] == AFFINITY_UNDEFINED)
                        aff[i] = 'A';
        }
-       aff[i] = 0;
+       aff[column_count] = '\0';
 
        return aff;
 }

> 
>> +	 *
>> +	 * The column affinity string will eventually be deleted by
>> +	 * sqliteDeleteIndex() when the Index structure itself is cleaned
>> +	 * up.
>> +	 */
>> +	int nColumn = def->key_def->part_count;
>> +	aff = (char *)sqlite3DbMallocRaw(0, nColumn + 1);
> 
> 9. Why do you need to declare 'char *aff;' separately above?

Fixed, see diff above.

> 
>> +	if (aff == NULL) {
>> +		sqlite3OomFault(db);
>> +		return 0;
>> +	}
>> +	int i;
> 
> 10. Can be part of 'for’.

Fixed, see diff above.

> 
>> +	struct space *space = space_cache_find(def->space_id);
>> +	assert(space != NULL);
>> +
>> +	for (i = 0; i < nColumn; i++) {
>> +		uint32_t x = def->key_def->parts[i].fieldno;
>> +		aff[i] = space->def->fields[x].affinity;
>> +		if (aff[i] == AFFINITY_UNDEFINED)
>> +			aff[i] = 'A';
>> +	}
>> +	aff[i] = 0;
>> +
>> +	return aff;
>> +}
>> +
>> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
>> index 943fda9..6ea956d 100644
>> --- a/src/box/sql/sqliteInt.h
>> +++ b/src/box/sql/sqliteInt.h
>> @@ -2536,6 +2536,8 @@ struct SrcList {
>>  		char *zName;	/* Name of the table */
>>  		char *zAlias;	/* The "B" part of a "A AS B" phrase.  zName is the "A" */
>>  		Table *pTab;	/* An SQL table corresponding to zName */
>> +		/* A temporary hack: need to store eph. space.  */
>> +		struct space *space;
> 
> 11. ??? It is unused.

Indeed, removed:

+++ b/src/box/sql/sqliteInt.h
@@ -2536,8 +2536,6 @@ struct SrcList {
                char *zName;    /* Name of the table */
                char *zAlias;   /* The "B" part of a "A AS B" phrase.  zName is the "A" */
                Table *pTab;    /* An SQL table corresponding to zName */
-               /* A temporary hack: need to store eph. space.  */
-               struct space *space;

> 
>>  		Select *pSelect;	/* A SELECT statement used in place of a table name */
>>  		int addrFillSub;	/* Address of subroutine to manifest a subquery */
>>  		int regReturn;	/* Register holding return address of addrFillSub */
>> diff --git a/src/box/sql/where.c b/src/box/sql/where.c
>> index d312587..14cb23d 100644
>> --- a/src/box/sql/where.c
>> +++ b/src/box/sql/where.c
>> @@ -371,7 +371,10 @@ whereScanInit(WhereScan * pScan,	/* The WhereScan object being initialized */
>>  	pScan->is_column_seen = false;
>>  	if (pIdx) {
>>  		int j = iColumn;
>> -		iColumn = pIdx->aiColumn[j];
>> +		if (j >= pIdx->nColumn)
>> +			iColumn = -1;
>> +		else
>> +			iColumn = pIdx->aiColumn[j];
> 
> 12. How can j can be > nColumn?

No way:

@@ -371,10 +372,7 @@ whereScanInit(WhereScan * pScan,   /* The WhereScan object being initialized */
        pScan->is_column_seen = false;
        if (pIdx) {
                int j = iColumn;
-               if (j >= pIdx->nColumn)
-                       iColumn = -1;
-               else
-                       iColumn = pIdx->aiColumn[j];
+               iColumn = pIdx->aiColumn[j];

> 
>>  		if (iColumn >= 0) {
>>  			char affinity =
>>  				pIdx->pTable->def->fields[iColumn].affinity;
>> @@ -441,6 +472,36 @@ sqlite3WhereFindTerm(WhereClause * pWC,	/* The WHERE clause to be searched */
>>  	return pResult;
>>  }
>>  +WhereTerm *
>> +sql_where_find_term(WhereClause * pWC,	/* The WHERE clause to be searched */
>> +		    int iCur,		/* Cursor number of LHS */
>> +		    int iColumn,	/* Column number of LHS */
>> +		    Bitmask notReady,	/* RHS must not overlap with this mask */
>> +		    u32 op,		/* Mask of WO_xx values describing operator */
>> +		    struct space_def *space_def,
>> +		    struct key_def *key_def)	/* Must be compatible with this index, if not NULL */
>> +{
>> +	WhereTerm *pResult = 0;
>> +	WhereTerm *p;
>> +	WhereScan scan;
>> +
>> +	p = sql_where_scan_init(&scan, pWC, iCur, iColumn, op, space_def,
>> +				key_def);
> 
> 13. Why do you need to declare 'WhereTerm *p;' above? Please, make the new functions
> obey Tarantool code style. I will not mention it again below.

Ok, I have refactored this function. See diff at the end of letter.

> 
>> +	op &= WO_EQ;
>> +	while (p) {
>> +		if ((p->prereqRight & notReady) == 0) {
>> +			if (p->prereqRight == 0 && (p->eOperator & op) != 0) {
>> +				testcase(p->eOperator & WO_IS);
>> +				return p;
>> +			}
>> +			if (pResult == 0)
>> +				pResult = p;
>> +		}
>> +		p = whereScanNext(&scan);
>> +	}
>> +	return pResult;
>> +}
>> +
>>  /*
>>   * This function searches pList for an entry that matches the iCol-th column
>>   * of index pIdx.
>> @@ -1703,6 +1764,7 @@ whereLoopInit(WhereLoop * p)
>>  	p->nLTerm = 0;
>>  	p->nLSlot = ArraySize(p->aLTermSpace);
>>  	p->wsFlags = 0;
>> +	p->index_def = NULL;
> 
> 14. Why for non-existing struct Table you have created dummy one, but for
> non-existing struct Index you can not do the same? You can add
> struct index_def to struct Index for this, it is not? Anyways index_def
> is going to be part of struct Index.

AFAIK Ivan works on this issue, lets wait for his patch.

> 
>>  }
>>    /*
>> @@ -3366,7 +3428,7 @@ 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) {
>> +				if (pIndex && j < pIndex->nColumn) {
> 
> 15. pIndex != NULL
> 
> 16. How j can be > pIndex->nColumn?

@@ -2668,7 +2650,8 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage)
-				if (pIndex && j < pIndex->nColumn) {
+				if (pIndex != NULL) {


> 
>>  					iColumn = pIndex->aiColumn[j];
>>  					revIdx = sql_index_column_sort_order(pIndex,
>>  									     j);
>> @@ -4080,34 +4142,74 @@ whereShortCut(WhereLoopBuilder * pBuilder)
>>  		/* TUNING: Cost of a PK lookup is 10 */
>>  		pLoop->rRun = 33;	/* 33==sqlite3LogEst(10) */
>>  	} else {
>> -		for (pIdx = pTab->pIndex; pIdx; pIdx = pIdx->pNext) {
>> +		struct space *space = space_cache_find(space_def->id);
>> +		if (space != NULL) {
>> +			for (uint32_t i = 0; i < space->index_count; ++i) {
>> +				struct index_def *idx_def;
>> +				idx_def = space->index[i]->def;
>> +				int opMask;
>> +				int nIdxCol = idx_def->key_def->part_count;
>> +				assert(pLoop->aLTermSpace == pLoop->aLTerm);
>> +				if (!idx_def->opts.is_unique
>> +				    /* || pIdx->pPartIdxWhere != 0 */
>> +				    || nIdxCol > ArraySize(pLoop->aLTermSpace)
>> +					)
>> +					continue;
>> +				opMask = WO_EQ;
>> +				for (j = 0; j < nIdxCol; j++) {
>> +					pTerm = sql_where_find_term(pWC, iCur,
>> +								    j, 0,
>> +								    opMask,
>> +								    space_def,
>> +								    idx_def->
>> +								    key_def);
>> +					if (pTerm == 0)
>> +						break;
>> +					testcase(pTerm->eOperator & WO_IS);
>> +					pLoop->aLTerm[j] = pTerm;
>> +				}
>> +				if (j != nIdxCol)
>> +					continue;
>> +				pLoop->wsFlags = WHERE_COLUMN_EQ |
>> +					WHERE_ONEROW | WHERE_INDEXED |
>> +					WHERE_IDX_ONLY;
>> +				pLoop->nLTerm = j;
>> +				pLoop->nEq = j;
>> +				pLoop->pIndex = NULL;
>> +				pLoop->index_def = idx_def;
>> +				/* TUNING: Cost of a unique index lookup is 15 */
>> +				pLoop->rRun = 39;	/* 39==sqlite3LogEst(15) */
>> +				break;
> 
> 17. Too much copypaste. Can you please reduce duplicating?

See patch below.

=======================================================================

From 9ced467a2f33ce02ac48f875db06d3eec7d5561d Mon Sep 17 00:00:00 2001
From: Nikita Pettik <korablev@tarantool.org>
Date: Mon, 18 Jun 2018 00:38:55 +0300
Subject: [PATCH] sql: review fixes for a40287051

---
 src/box/sql/analyze.c   |   6 +-
 src/box/sql/build.c     |  27 +----
 src/box/sql/delete.c    |  45 +++----
 src/box/sql/expr.c      |   9 +-
 src/box/sql/fkey.c      |   7 +-
 src/box/sql/insert.c    |  36 +++---
 src/box/sql/resolve.c   |  18 +++
 src/box/sql/select.c    |   5 +-
 src/box/sql/sqliteInt.h |  42 ++++---
 src/box/sql/where.c     | 309 +++++++++++++++++++++++++-----------------------
 src/box/sql/wherecode.c |   5 +-
 11 files changed, 256 insertions(+), 253 deletions(-)

diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 9509645c0..28d68a55b 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -176,9 +176,9 @@ openStatTable(Parse * pParse,	/* Parsing context */
 
 	/* Open the sql_stat[134] tables for writing. */
 	for (i = 0; aTable[i]; i++) {
-		int addr = emit_open_cursor(pParse, iStatCur + i, aRoot[i]);
-		v->aOp[addr].p4.key_def = NULL;
-		v->aOp[addr].p4type = P4_KEYDEF;
+		struct space *space =
+			space_by_id(SQLITE_PAGENO_TO_SPACEID(aRoot[i]));
+		sql_emit_open_cursor(pParse, iStatCur + i, aRoot[i], space);
 		sqlite3VdbeChangeP5(v, aCreateTbl[i]);
 		VdbeComment((v, aTable[i]));
 	}
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 552578048..8c83a797e 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1181,35 +1181,17 @@ space_checks_expr_list(uint32_t space_id)
 }
 
 int
-emit_open_cursor(struct Parse *parse_context, int cursor, int entity_id)
+sql_emit_open_cursor(struct Parse *parse_context, int cursor, int index_id,
+		     struct space *space)
 {
-	assert(entity_id > 0);
-	struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(entity_id));
 	assert(space != NULL);
 	struct Vdbe *vdbe = parse_context->pVdbe;
 	int space_ptr_reg = ++parse_context->nMem;
 	sqlite3VdbeAddOp4(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
 			  P4_SPACEPTR);
-	return sqlite3VdbeAddOp3(vdbe, OP_OpenWrite, cursor, entity_id,
+	return sqlite3VdbeAddOp3(vdbe, OP_OpenWrite, cursor, index_id,
 				 space_ptr_reg);
 }
-
-int
-sql_emit_open_cursor(struct Parse *parse, int cursor, int index_id, struct space *space)
-{
-	assert(space != NULL);
-	Vdbe *vdbe = parse->pVdbe;
-	int space_ptr_reg = ++parse->nMem;
-	sqlite3VdbeAddOp4(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
-			  P4_SPACEPTR);
-	struct key_def *def = key_def_dup(space->index[index_id]->def->key_def);
-	if (def == NULL)
-		return 0;
-	return sqlite3VdbeAddOp4(vdbe, OP_OpenWrite, cursor, index_id,
-				 space_ptr_reg,
-				 (char*)def,
-				 P4_KEYDEF);
-}
 /*
  * Generate code that will increment the schema cookie.
  *
@@ -2668,7 +2650,8 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage)
 	if (memRootPage < 0)
 		sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum),
 				  0);
-	emit_open_cursor(pParse, iIdx, tnum);
+	struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(tnum));
+	sql_emit_open_cursor(pParse, iIdx, tnum, space);
 	sqlite3VdbeChangeP5(v,
 			    OPFLAG_BULKCSR | ((memRootPage >= 0) ?
 					      OPFLAG_P2ISREG : 0));
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 23e2bcc49..1beacec48 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -88,7 +88,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 	 * instead of just a Table* parameter.
 	 */
 	struct Table *table = NULL;
-	struct space *space;
+	struct space *space = NULL;
 	uint32_t space_id;
 	/* Figure out if we have any triggers and if the table
 	 * being deleted from is a view.
@@ -104,17 +104,28 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 						strlen(tab_name));
 		if (space_id == BOX_ID_NIL)
 			goto delete_from_cleanup;
+		/*
+		 * In this case space has been created via Lua
+		 * facilities, so there is no corresponding entry
+		 * in table hash. Thus, lets create simple
+		 * wrapper around space_def to support interface.
+		 */
+		space = space_by_id(space_id);
+		tab_list->a[0].pTab = sql_table_construct_from_space(space);
+		if (tab_list->a[0].pTab == NULL)
+			goto delete_from_cleanup;
 	} else {
 		table = sql_list_lookup_table(parse, tab_list);
 		if (table == NULL)
 			goto delete_from_cleanup;
 		space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
+		space = space_by_id(space_id);
+		assert(space != NULL);
 		trigger_list =sqlite3TriggersExist(table, TK_DELETE,
 						   NULL, NULL);
 		is_complex = trigger_list != NULL ||
 			     sqlite3FkRequired(table, NULL);
 	}
-	space = space_by_id(space_id);
 	assert(space != NULL);
 
 	bool is_view = space->def->opts.is_view;
@@ -183,16 +194,6 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 		struct NameContext nc;
 		memset(&nc, 0, sizeof(nc));
 		nc.pParse = parse;
-		if (tab_list->a[0].pTab == NULL) {
-			struct Table *t = calloc(sizeof(struct Table), 1);
-			if (t == NULL) {
-				sqlite3OomFault(parse->db);
-				goto delete_from_cleanup;
-			}
-			assert(space != NULL);
-			t->def = space->def;
-			tab_list->a[0].pTab = t;
-		}
 		nc.pSrcList = tab_list;
 		if (sqlite3ResolveExprNames(&nc, where))
 			goto delete_from_cleanup;
@@ -266,11 +267,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 		/* Extract the primary key for the current row */
 		if (!is_view) {
 			for (int i = 0; i < pk_len; i++) {
-				struct space_def *def;
-				if (table != NULL)
-					def = table->def;
-				else
-					def = space->def;
+				struct space_def *def = space->def;
 				sqlite3ExprCodeGetColumnOfTable(v, def,
 								tab_cursor,
 								pk_def->parts[i].fieldno,
@@ -342,15 +339,9 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 			sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
 					  (void *)space, P4_SPACEPTR);
 
-			int tnum;
-			if (table != NULL) {
-				tnum = table->tnum;
-			}
-			else {
-				/* index id is 0 for PK.  */
-				tnum = SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(space->def->id,
-									      0);
-			}
+			int tnum =
+				SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(space->def->id,
+								       0);
 			sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor,
 					  tnum, space_ptr_reg);
 			struct key_def *def = key_def_dup(pk_def);
@@ -518,7 +509,7 @@ sql_generate_row_delete(struct Parse *parse, struct Table *table,
 	 * of the DELETE statement is to fire the INSTEAD OF
 	 * triggers).
 	 */
-	if (table == NULL || table->pSelect == NULL) {
+	if (table == NULL || !table->def->opts.is_view) {
 		uint8_t p5 = 0;
 		sqlite3VdbeAddOp2(v, OP_Delete, cursor,
 				  (need_update_count ? OPFLAG_NCHANGE : 0));
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index a69d38bd0..1a5e130d0 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -36,6 +36,8 @@
 #include "box/coll_id_cache.h"
 #include "coll.h"
 #include "sqliteInt.h"
+#include "tarantoolInt.h"
+#include "box/schema.h"
 #include "box/session.h"
 
 /* Forward declarations */
@@ -2485,9 +2487,10 @@ sqlite3FindInIndex(Parse * pParse,	/* Parsing context */
 							  "USING INDEX %s FOR IN-OPERATOR",
 							  pIdx->zName),
 							  P4_DYNAMIC);
-					emit_open_cursor(pParse, iTab,
-							 pIdx->tnum);
-					sql_vdbe_set_p4_key_def(pParse, pIdx);
+					struct space *space =
+						space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
+					sql_emit_open_cursor(pParse, iTab,
+							     pIdx->tnum, space);
 					VdbeComment((v, "%s", pIdx->zName));
 					assert(IN_INDEX_INDEX_DESC ==
 					       IN_INDEX_INDEX_ASC + 1);
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index 6fd29200c..74cf51302 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -35,6 +35,7 @@
  */
 #include "coll.h"
 #include "sqliteInt.h"
+#include "box/schema.h"
 #include "box/session.h"
 #include "tarantoolInt.h"
 
@@ -439,9 +440,9 @@ fkLookupParent(Parse * pParse,	/* Parse context */
 			int nCol = pFKey->nCol;
 			int regTemp = sqlite3GetTempRange(pParse, nCol);
 			int regRec = sqlite3GetTempReg(pParse);
-
-			emit_open_cursor(pParse, iCur, pIdx->tnum);
-			sql_vdbe_set_p4_key_def(pParse, pIdx);
+			struct space *space =
+				space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
+			sql_emit_open_cursor(pParse, iCur, pIdx->tnum, space);
 			for (i = 0; i < nCol; i++) {
 				sqlite3VdbeAddOp2(v, OP_Copy,
 						  aiCol[i] + 1 + regData,
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 7aba83529..0a0b3fc21 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -54,8 +54,8 @@ sqlite3OpenTable(Parse * pParse,	/* Generate code into this VDBE */
 	Index *pPk = sqlite3PrimaryKeyIndex(pTab);
 	assert(pPk != 0);
 	assert(pPk->tnum == pTab->tnum);
-	emit_open_cursor(pParse, iCur, pPk->tnum);
-	sql_vdbe_set_p4_key_def(pParse, pPk);
+	struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pPk->tnum));
+	sql_emit_open_cursor(pParse, iCur, pPk->tnum, space);
 	VdbeComment((v, "%s", pTab->def->name));
 }
 
@@ -107,32 +107,22 @@ sqlite3IndexAffinityStr(sqlite3 *db, Index *index)
 char *
 sql_index_affinity_str(struct sqlite3 * db, struct index_def *def)
 {
-	char *aff;
-	/* The first time a column affinity string for a particular index is
-	 * required, it is allocated and populated here. It is then stored as
-	 * a member of the Index structure for subsequent use.
-	 *
-	 * The column affinity string will eventually be deleted by
-	 * sqliteDeleteIndex() when the Index structure itself is cleaned
-	 * up.
-	 */
-	int nColumn = def->key_def->part_count;
-	aff = (char *)sqlite3DbMallocRaw(0, nColumn + 1);
+	uint32_t column_count = def->key_def->part_count;
+	char *aff = (char *)sqlite3DbMallocRaw(0, column_count + 1);
 	if (aff == NULL) {
 		sqlite3OomFault(db);
-		return 0;
+		return NULL;
 	}
-	int i;
-	struct space *space = space_cache_find(def->space_id);
+	struct space *space = space_by_id(def->space_id);
 	assert(space != NULL);
 
-	for (i = 0; i < nColumn; i++) {
+	for (uint32_t i = 0; i < column_count; i++) {
 		uint32_t x = def->key_def->parts[i].fieldno;
 		aff[i] = space->def->fields[x].affinity;
 		if (aff[i] == AFFINITY_UNDEFINED)
 			aff[i] = 'A';
 	}
-	aff[i] = 0;
+	aff[column_count] = '\0';
 
 	return aff;
 }
@@ -1951,11 +1941,13 @@ xferOptimization(Parse * pParse,	/* Parser context */
 				break;
 		}
 		assert(pSrcIdx);
-		emit_open_cursor(pParse, iSrc, pSrcIdx->tnum);
-		sql_vdbe_set_p4_key_def(pParse, pSrcIdx);
+		struct space *src_space =
+			space_by_id(SQLITE_PAGENO_TO_SPACEID(pSrcIdx->tnum));
+		sql_emit_open_cursor(pParse, iSrc, pSrcIdx->tnum, src_space);
 		VdbeComment((v, "%s", pSrcIdx->zName));
-		emit_open_cursor(pParse, iDest, pDestIdx->tnum);
-		sql_vdbe_set_p4_key_def(pParse, pDestIdx);
+		struct space *dest_space =
+			space_by_id(SQLITE_PAGENO_TO_SPACEID(pDestIdx->tnum));
+		sql_emit_open_cursor(pParse, iDest, pDestIdx->tnum, dest_space);
 		sqlite3VdbeChangeP5(v, OPFLAG_BULKCSR);
 		VdbeComment((v, "%s", pDestIdx->zName));
 		addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iSrc, 0);
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 23e16189a..e7d9723fa 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -39,6 +39,9 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "box/box.h"
+#include "box/schema.h"
+
 /*
  * Walk the expression tree pExpr and increase the aggregate function
  * depth (the Expr.op2 field) by N on every TK_AGG_FUNCTION node.
@@ -1432,6 +1435,21 @@ resolveSelectStep(Walker * pWalker, Select * p)
 	return WRC_Prune;
 }
 
+struct Table *
+sql_table_construct_from_space(const struct space *space)
+{
+	assert(space != NULL);
+	struct Table *table = calloc(1, sizeof(*table));
+	if (table == NULL) {
+		diag_set(OutOfMemory, sizeof(table), "calloc", "table");
+		return NULL;
+	}
+	table->def = space_def_dup(space->def);
+	if (table->def == NULL)
+		return NULL;
+	return table;
+}
+
 /*
  * This routine walks an expression tree and resolves references to
  * table columns and result-set columns.  At the same time, do error
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 4b5ba4d3e..9e8ed97e7 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -6107,8 +6107,9 @@ sqlite3Select(Parse * pParse,		/* The parser context */
 				 * Open the cursor, execute the OP_Count,
 				 * close the cursor.
 				 */
-				emit_open_cursor(pParse, cursor,
-						 space->def->id << 10);
+				sql_emit_open_cursor(pParse, cursor,
+						     space->def->id << 10,
+						     space);
 				sqlite3VdbeAddOp2(v, OP_Count, cursor,
 						  sAggInfo.aFunc[0].iMem);
 				sqlite3VdbeAddOp1(v, OP_Close, cursor);
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 878daa8df..b7e7eafa8 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2536,8 +2536,6 @@ struct SrcList {
 		char *zName;	/* Name of the table */
 		char *zAlias;	/* The "B" part of a "A AS B" phrase.  zName is the "A" */
 		Table *pTab;	/* An SQL table corresponding to zName */
-		/* A temporary hack: need to store eph. space.  */
-		struct space *space;
 		Select *pSelect;	/* A SELECT statement used in place of a table name */
 		int addrFillSub;	/* Address of subroutine to manifest a subquery */
 		int regReturn;	/* Register holding return address of addrFillSub */
@@ -3553,20 +3551,6 @@ sql_index_column_sort_order(Index *idx, uint32_t column);
 
 void sqlite3EndTable(Parse *, Token *, Token *, Select *);
 
-/**
- * DEPRECATED. All calls to be replaced w/ sql_emit_open_cursor.
- * Create cursor which will be positioned to the space/index.
- * It makes space lookup and loads pointer to it into register,
- * which is passes to OP_OpenWrite as an argument.
- *
- * @param parse Parse context.
- * @param cursor Number of cursor to be created.
- * @param entity_id Encoded space and index ids.
- * @retval address of last opcode.
- */
-int
-emit_open_cursor(struct Parse *parse, int cursor, int entity_id);
-
 /**
  * Create cursor which will be positioned to the space/index.
  * It makes space lookup and loads pointer to it into register,
@@ -4125,9 +4109,10 @@ int sqlite3VarintLen(u64 v);
 const char *sqlite3IndexAffinityStr(sqlite3 *, Index *);
 
 /**
- * Return a pointer to the column affinity string associated with index
- * pIdx. A column affinity string has one character for each column in
- * the table, according to the affinity of the column:
+ * Return a pointer to the column affinity string associated with
+ * given index. A column affinity string has one character for
+ * each column in the table, according to the affinity of the
+ * column:
  *
  *  Character      Column affinity
  *  ------------------------------
@@ -4138,12 +4123,11 @@ const char *sqlite3IndexAffinityStr(sqlite3 *, Index *);
  *  'F'            REAL
  *
  * Memory for the buffer containing the column index affinity string
- * is managed along with the rest of the Index structure. It will be
- * released when sqlite3DeleteIndex() is called.
+ * is allocated on heap.
  *
  * @param db Database handle.
  * @param def index_def where from affinity to be extracted.
- * @retval Affinity string.
+ * @retval Allocated affinity string, or NULL on OOM.
  */
 char *
 sql_index_affinity_str(struct sqlite3 *db, struct index_def *def);
@@ -4262,6 +4246,20 @@ void
 sqlite3SelectWrongNumTermsError(struct Parse *parse, struct Select *p);
 
 int sqlite3MatchSpanName(const char *, const char *, const char *);
+
+/**
+ * Create wrapper around space_def of the given space.
+ * This routine is required to support original SQLite interface,
+ * which accepts struct Table and struct Index DD arguments.
+ * Both table and space def are allocated on heap.
+ *
+ * @param space Space to be prototype.
+ * @retval New instance of struct Table allocated on malloc,
+ *         or NULL in case of OOM.
+ */
+struct Table *
+sql_table_construct_from_space(const struct space *space);
+
 int sqlite3ResolveExprNames(NameContext *, Expr *);
 int sqlite3ResolveExprListNames(NameContext *, ExprList *);
 void sqlite3ResolveSelectNames(Parse *, Select *, NameContext *);
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index f017384b5..a3a5bec81 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -42,6 +42,7 @@
 #include "tarantoolInt.h"
 #include "vdbeInt.h"
 #include "whereInt.h"
+#include "box/coll_id_cache.h"
 #include "box/session.h"
 #include "box/schema.h"
 
@@ -371,10 +372,7 @@ whereScanInit(WhereScan * pScan,	/* The WhereScan object being initialized */
 	pScan->is_column_seen = false;
 	if (pIdx) {
 		int j = iColumn;
-		if (j >= pIdx->nColumn)
-			iColumn = -1;
-		else
-			iColumn = pIdx->aiColumn[j];
+		iColumn = pIdx->aiColumn[j];
 		if (iColumn >= 0) {
 			char affinity =
 				pIdx->pTable->def->fields[iColumn].affinity;
@@ -393,12 +391,30 @@ whereScanInit(WhereScan * pScan,	/* The WhereScan object being initialized */
 	return whereScanNext(pScan);
 }
 
+/**
+ * Analogue of whereScanInit() but also can be called for spaces
+ * created via Lua interface. This function doesn't rely on
+ * regular SQLite structures representing data dictionary.
+ *
+ * @param scan The WhereScan object being initialized.
+ * @param clause The WHERE clause to be scanned.
+ * @param cursor Cursor to scan for.
+ * @param column Column to scan for.
+ * @param op_mask Operator(s) to scan for.
+ * @param space_def Def of the space related to WHERE clause.
+ * @param key_def Def of the index to be used to satisfy WHERE
+ *                clause. May be NULL.
+ *
+ * @retval Return a pointer to the first match. Return NULL if
+ *         there are no matches.
+ */
 static WhereTerm *
-sql_where_scan_init(struct WhereScan *scan, struct WhereClause *clause,
+where_scan_init(struct WhereScan *scan, struct WhereClause *clause,
 		    int cursor, int column, uint32_t op_mask,
 		    struct space_def *space_def, struct key_def *key_def)
 {
-	scan->pOrigWC = scan->pWC = clause;
+	scan->pOrigWC = clause;
+	scan->pWC = clause;
 	scan->pIdxExpr = NULL;
 	scan->idxaff = 0;
 	scan->coll = NULL;
@@ -406,11 +422,11 @@ sql_where_scan_init(struct WhereScan *scan, struct WhereClause *clause,
 	if (key_def != NULL) {
 		int j = column;
 		column = key_def->parts[j].fieldno;
-		if (column >= 0) {
-			scan->idxaff = space_def->fields[column].affinity;
-			scan->coll = key_def->parts[column].coll;
-			scan->is_column_seen = true;
-		}
+		scan->idxaff = space_def->fields[column].affinity;
+		uint32_t coll_id = space_def->fields[column].coll_id;
+		struct coll_id *coll = coll_by_id(coll_id);
+		scan->coll = coll != NULL ? coll->coll : NULL;
+		scan->is_column_seen = true;
 	}
 	scan->opMask = op_mask;
 	scan->k = 0;
@@ -472,34 +488,43 @@ sqlite3WhereFindTerm(WhereClause * pWC,	/* The WHERE clause to be searched */
 	return pResult;
 }
 
+/**
+ * Analogue of sqlite3WhereFindTerm() but also can be called
+ * for spaces created via Lua interface. This function doesn't
+ * rely on regular SQLite structures representing data
+ * dictionary.
+ *
+ * @param where_clause The WHERE clause to be examined.
+ * @param cursor Cursor number of LHS.
+ * @param column Column number of LHS
+ * @param is_ready RHS must not overlap with this mask.
+ * @param op Mask of WO_xx values describing operator.
+ * @param space_def Def of the space related to WHERE clause.
+ * @param key_def Def of the index to be used to satisfy WHERE
+ *                clause. May be NULL.
+ *
+ * @retval New struct describing WHERE term.
+ */
 WhereTerm *
-sql_where_find_term(WhereClause * pWC,	/* The WHERE clause to be searched */
-		    int iCur,		/* Cursor number of LHS */
-		    int iColumn,	/* Column number of LHS */
-		    Bitmask notReady,	/* RHS must not overlap with this mask */
-		    u32 op,		/* Mask of WO_xx values describing operator */
-		    struct space_def *space_def,
-		    struct key_def *key_def)	/* Must be compatible with this index, if not NULL */
+sql_where_find_term(WhereClause *where_clause, int cursor, int column,
+		    Bitmask is_ready, u32 op, struct space_def *space_def,
+		    struct key_def *key_def)
 {
-	WhereTerm *pResult = 0;
-	WhereTerm *p;
+	WhereTerm *result = NULL;
 	WhereScan scan;
-
-	p = sql_where_scan_init(&scan, pWC, iCur, iColumn, op, space_def,
-				key_def);
+	WhereTerm *p = where_scan_init(&scan, where_clause, cursor, column,
+					   op, space_def, key_def);
 	op &= WO_EQ;
-	while (p) {
-		if ((p->prereqRight & notReady) == 0) {
-			if (p->prereqRight == 0 && (p->eOperator & op) != 0) {
-				testcase(p->eOperator & WO_IS);
+	while (p != NULL) {
+		if ((p->prereqRight & is_ready) == 0) {
+			if (p->prereqRight == 0 && (p->eOperator & op) != 0)
 				return p;
-			}
-			if (pResult == 0)
-				pResult = p;
+			if (result == NULL)
+				result = p;
 		}
 		p = whereScanNext(&scan);
 	}
-	return pResult;
+	return result;
 }
 
 /*
@@ -3428,7 +3453,7 @@ 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 && j < pIndex->nColumn) {
+				if (pIndex != NULL) {
 					iColumn = pIndex->aiColumn[j];
 					revIdx = sql_index_column_sort_order(pIndex,
 									     j);
@@ -4096,135 +4121,125 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
 	return SQLITE_OK;
 }
 
-/*
- * Most queries use only a single table (they are not joins) and have
- * simple == constraints against indexed fields.  This routine attempts
- * to plan those simple cases using much less ceremony than the
- * general-purpose query planner, and thereby yield faster sqlite3_prepare()
- * times for the common case.
+/**
+ * Attempt at finding appropriate terms in WHERE clause.
  *
- * Return non-zero on success, if this query can be handled by this
- * no-frills query planner.  Return zero if this query needs the
- * general-purpose query planner.
+ * @param loop The loop @where belongs to.
+ * @param where The WHERE clause to be examined..
+ * @param cursor Cursor number of LHS.
+ * @param space_def Def of the space related to WHERE clause.
+ * @param index_def Def of the index to be used to satisfy WHERE
+ *                  clause. May be NULL.
+ * @retval 0 on success, -1 otherwise.
  */
 static int
-sql_where_shortcut(struct WhereLoopBuilder *builder)
+where_assign_loop_terms(struct WhereLoop *loop, struct WhereClause *where,
+			int cursor, struct space_def *space_def,
+			struct index_def *idx_def)
 {
-	WhereInfo *pWInfo;
-	struct SrcList_item *pItem;
-	WhereClause *pWC;
-	WhereTerm *pTerm;
-	WhereLoop *pLoop;
-	int iCur;
-	int j;
+	uint32_t column_count = idx_def != NULL ? idx_def->key_def->part_count :
+				space_def->field_count;
+	if (column_count > ArraySize(loop->aLTermSpace))
+		return -1;
+	uint32_t i;
+	for (i = 0; i < column_count; ++i) {
+		struct WhereTerm *term =
+			sql_where_find_term(where, cursor, i, 0, WO_EQ,
+					    space_def, idx_def != NULL ?
+					    idx_def->key_def : NULL);
+		if (term == NULL)
+			break;
+		testcase(pTerm->eOperator & WO_IS);
+		loop->aLTerm[i] = term;
+	}
+	if (i != column_count)
+		return -1;
+	loop->wsFlags = WHERE_COLUMN_EQ | WHERE_ONEROW | WHERE_INDEXED |
+			WHERE_IDX_ONLY;
+	loop->nLTerm = i;
+	loop->nEq = i;
+	loop->index_def = idx_def;
+	/* TUNING: Cost of a unique index lookup is 15. */
+	assert(39 == sqlite3LogEst(15));
+	loop->rRun = 39;
+	return 0;
+}
 
-	pWInfo = builder->pWInfo;
-	if (pWInfo->wctrlFlags & WHERE_OR_SUBCLAUSE)
+/**
+ * Most queries use only a single table (they are not joins) and
+ * have simple == constraints against indexed fields. This
+ * routine attempts to plan those simple cases using much less
+ * ceremony than the general-purpose query planner, and thereby
+ * yield faster sqlite3_prepare() times for the common case.
+ *
+ * @param builder Where-Loop Builder.
+ * @retval Return non-zero on success, i.e. if this query can be
+ *         handled by this no-frills query planner. Return zero
+ *         if this query needs the general-purpose query planner.
+ */
+static int
+sql_where_shortcut(struct WhereLoopBuilder *builder)
+{
+	struct WhereInfo *where_info = builder->pWInfo;
+	if (where_info->wctrlFlags & WHERE_OR_SUBCLAUSE)
 		return 0;
-	assert(pWInfo->pTabList->nSrc >= 1);
-	pItem = pWInfo->pTabList->a;
-	struct space_def *space_def = pItem->pTab->def;
+	assert(where_info->pTabList->nSrc >= 1);
+	struct SrcList_item *item = where_info->pTabList->a;
+	struct space_def *space_def = item->pTab->def;
 	assert(space_def != NULL);
-
-	if (pItem->fg.isIndexedBy)
+	if (item->fg.isIndexedBy)
 		return 0;
-	iCur = pItem->iCursor;
-	pWC = &pWInfo->sWC;
-	pLoop = builder->pNew;
-	pLoop->wsFlags = 0;
-	pLoop->nSkip = 0;
-	pTerm = sqlite3WhereFindTerm(pWC, iCur, -1, 0, WO_EQ, 0);
-	if (pTerm) {
-		pLoop->wsFlags = WHERE_COLUMN_EQ | WHERE_IPK | WHERE_ONEROW;
-		pLoop->aLTerm[0] = pTerm;
-		pLoop->nLTerm = 1;
-		pLoop->nEq = 1;
-		/* TUNING: Cost of a PK lookup is 10 */
-		pLoop->rRun = 33;	/* 33==sqlite3LogEst(10) */
+	int cursor = item->iCursor;
+	struct WhereClause *clause = &where_info->sWC;
+	struct WhereLoop *loop = builder->pNew;
+	loop->wsFlags = 0;
+	loop->nSkip = 0;
+	loop->pIndex = NULL;
+	struct WhereTerm *term = sqlite3WhereFindTerm(clause, cursor, -1, 0,
+						      WO_EQ, 0);
+	if (term != NULL) {
+		loop->wsFlags = WHERE_COLUMN_EQ | WHERE_IPK | WHERE_ONEROW;
+		loop->aLTerm[0] = term;
+		loop->nLTerm = 1;
+		loop->nEq = 1;
+		/* TUNING: Cost of a PK lookup is 10. */
+		assert(33 == sqlite3LogEst(10));
+		loop->rRun = 33;
 	} else {
-		struct space *space = space_cache_find(space_def->id);
+		assert(loop->aLTermSpace == loop->aLTerm);
+		struct space *space = space_by_id(space_def->id);
 		if (space != NULL) {
 			for (uint32_t i = 0; i < space->index_count; ++i) {
-				struct index_def *idx_def;
-				idx_def = space->index[i]->def;
-				int opMask;
-				int nIdxCol = idx_def->key_def->part_count;
-				assert(pLoop->aLTermSpace == pLoop->aLTerm);
-				if (!idx_def->opts.is_unique
-				    /* || pIdx->pPartIdxWhere != 0 */
-				    || nIdxCol > ArraySize(pLoop->aLTermSpace)
-					)
+				struct index_def *idx_def =
+					space->index[i]->def;
+				if (!idx_def->opts.is_unique)
 					continue;
-				opMask = WO_EQ;
-				for (j = 0; j < nIdxCol; j++) {
-					pTerm = sql_where_find_term(pWC, iCur,
-								    j, 0,
-								    opMask,
-								    space_def,
-								    idx_def->
-								    key_def);
-					if (pTerm == 0)
-						break;
-					testcase(pTerm->eOperator & WO_IS);
-					pLoop->aLTerm[j] = pTerm;
-				}
-				if (j != nIdxCol)
-					continue;
-				pLoop->wsFlags = WHERE_COLUMN_EQ |
-					WHERE_ONEROW | WHERE_INDEXED |
-					WHERE_IDX_ONLY;
-				pLoop->nLTerm = j;
-				pLoop->nEq = j;
-				pLoop->pIndex = NULL;
-				pLoop->index_def = idx_def;
-				/* TUNING: Cost of a unique index lookup is 15 */
-				pLoop->rRun = 39;	/* 39==sqlite3LogEst(15) */
-				break;
+				if (where_assign_loop_terms(loop, clause,
+							    cursor, space_def,
+							    idx_def) == 0)
+					break;
 			}
 		} else {
-			/* Space is ephemeral.  */
+			/* Space is ephemeral. */
 			assert(space_def->id == 0);
-			int opMask;
-			int nIdxCol = space_def->field_count;
-			assert(pLoop->aLTermSpace == pLoop->aLTerm);
-			if ( nIdxCol > ArraySize(pLoop->aLTermSpace))
-				return 0;
-			opMask = WO_EQ;
-			for (j = 0; j < nIdxCol; j++) {
-				pTerm = sql_where_find_term(pWC, iCur,
-							    j, 0,
-							    opMask,
-							    space_def,
-							    NULL);
-				if (pTerm == NULL)
-					break;
-				pLoop->aLTerm[j] = pTerm;
-			}
-			if (j != nIdxCol)
-				return 0;
-			pLoop->wsFlags = WHERE_COLUMN_EQ | WHERE_ONEROW |
-					 WHERE_INDEXED | WHERE_IDX_ONLY;
-			pLoop->nLTerm = j;
-			pLoop->nEq = j;
-			pLoop->pIndex = NULL;
-			pLoop->index_def = NULL;
-			/* TUNING: Cost of a unique index lookup is 15 */
-			pLoop->rRun = 39;	/* 39==sqlite3LogEst(15) */
+			where_assign_loop_terms(loop, clause, cursor,
+						space_def, NULL);
 		}
 	}
-	if (pLoop->wsFlags) {
-		pLoop->nOut = (LogEst) 1;
-		pWInfo->a[0].pWLoop = pLoop;
-		pLoop->maskSelf = sqlite3WhereGetMask(&pWInfo->sMaskSet, iCur);
-		pWInfo->a[0].iTabCur = iCur;
-		pWInfo->nRowOut = 1;
-		if (pWInfo->pOrderBy)
-			pWInfo->nOBSat = pWInfo->pOrderBy->nExpr;
-		if (pWInfo->wctrlFlags & WHERE_WANT_DISTINCT) {
-			pWInfo->eDistinct = WHERE_DISTINCT_UNIQUE;
+	if (loop->wsFlags) {
+		loop->nOut = (LogEst) 1;
+		where_info->a[0].pWLoop = loop;
+		loop->maskSelf = sqlite3WhereGetMask(&where_info->sMaskSet,
+						     cursor);
+		where_info->a[0].iTabCur = cursor;
+		where_info->nRowOut = 1;
+		if (where_info->pOrderBy)
+			where_info->nOBSat = where_info->pOrderBy->nExpr;
+		if (where_info->wctrlFlags & WHERE_WANT_DISTINCT) {
+			where_info->eDistinct = WHERE_DISTINCT_UNIQUE;
 		}
 #ifdef SQLITE_DEBUG
-		pLoop->cId = '0';
+		loop->cId = '0';
 #endif
 		return 1;
 	}
@@ -4704,8 +4719,7 @@ sqlite3WhereBegin(Parse * pParse,	/* The parser context */
 			 */
 			if (idx_def == NULL && pIx == NULL) continue;
 			bool is_primary = (pIx != NULL && IsPrimaryKeyIndex(pIx)) ||
-					   (idx_def != NULL && (idx_def->iid == 0)) |
-				(idx_def == NULL && pIx == NULL);
+					   (idx_def != NULL && (idx_def->iid == 0));
 			if (is_primary
 			    && (wctrlFlags & WHERE_OR_SUBCLAUSE) != 0) {
 				/* This is one term of an OR-optimization using
@@ -4752,9 +4766,10 @@ sqlite3WhereBegin(Parse * pParse,	/* The parser context */
 			assert(iIndexCur >= 0);
 			if (op) {
 				if (pIx != NULL) {
-					emit_open_cursor(pParse, iIndexCur,
-							 pIx->tnum);
-					sql_vdbe_set_p4_key_def(pParse, pIx);
+					struct space *space =
+						space_by_id(SQLITE_PAGENO_TO_SPACEID(pIx->tnum));
+					sql_emit_open_cursor(pParse, iIndexCur,
+							     pIx->tnum, space);
 				} else {
 					sql_emit_open_cursor(pParse, iIndexCur,
 							     idx_def->iid,
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 60e0ac7e4..eaab0b657 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -87,7 +87,7 @@ explainAppendTerm(StrAccum * pStr,	/* The text expression being built */
 			assert(def != NULL);
                         struct space *space = space_cache_find(def->space_id);
                         assert(space != NULL);
-                        name = space->def->fields[i].name;
+                        name = space->def->fields[i + iTerm].name;
 		}
 		sqlite3StrAccumAppendAll(pStr, name);
 	}
@@ -143,7 +143,8 @@ explainIndexRange(StrAccum * pStr, WhereLoop * pLoop)
 		} else {
 			struct space *space = space_cache_find(def->space_id);
 			assert(space != NULL);
-			z = space->def->fields[i].name;
+			uint32_t fieldno = def->key_def->parts[i].fieldno;
+			z = space->def->fields[fieldno].name;
 		}
 		if (i)
 			sqlite3StrAccumAppend(pStr, " AND ", 5);
-- 
2.15.1