sql: review fixes for a402870

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]));
+		vdbe_emit_open_cursor(pParse, iStatCur + i, aRoot[i], space);
 		sqlite3VdbeChangeP5(v, aCreateTbl[i]);
 		VdbeComment((v, aTable[i]));
 	}

src/box/sql/build.c

@@ -1177,35 +1177,17 @@ space_checks_expr_list(uint32_t space_id)
 }
 
 int
-emit_open_cursor(struct Parse *parse_context, int cursor, int entity_id)
+vdbe_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.
  *
@@ -2664,7 +2646,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));
+	vdbe_emit_open_cursor(pParse, iIdx, tnum, space);
 	sqlite3VdbeChangeP5(v,
 			    OPFLAG_BULKCSR | ((memRootPage >= 0) ?
 					      OPFLAG_P2ISREG : 0));

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.
@@ -99,22 +99,36 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 	 */
 	bool is_complex = false;
 	const char *tab_name = tab_list->a->zName;
+	struct Table tmp_tab;
 	if (sqlite3LocateTable(parse, LOCATE_NOERR, tab_name) == NULL) {
 		space_id = box_space_id_by_name(tab_name,
 						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);
+		memset(&tmp_tab, 0, sizeof(tmp_tab));
+		tmp_tab.def = space->def;
+		/* Prevent from freeing memory in DeleteTable. */
+		tmp_tab.nTabRef = 2;
+		tab_list->a[0].pTab = &tmp_tab;
 	} 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 +197,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 +270,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 +342,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_id,
+								       0);
 			sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor,
 					  tnum, space_ptr_reg);
 			struct key_def *def = key_def_dup(pk_def);
@@ -518,7 +512,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));

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));
+					vdbe_emit_open_cursor(pParse, iTab,
+							      pIdx->tnum, space);
 					VdbeComment((v, "%s", pIdx->zName));
 					assert(IN_INDEX_INDEX_DESC ==
 					       IN_INDEX_INDEX_ASC + 1);

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));
+			vdbe_emit_open_cursor(pParse, iCur, pIdx->tnum, space);
 			for (i = 0; i < nCol; i++) {
 				sqlite3VdbeAddOp2(v, OP_Copy,
 						  aiCol[i] + 1 + regData,

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));
+	vdbe_emit_open_cursor(pParse, iCur, pPk->tnum, space);
 	VdbeComment((v, "%s", pTab->def->name));
 }
 
@@ -105,34 +105,22 @@ sqlite3IndexAffinityStr(sqlite3 *db, Index *index)
 }
 
 char *
-sql_index_affinity_str(struct sqlite3 * db, struct index_def *def)
+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);
-	if (aff == NULL) {
-		sqlite3OomFault(db);
-		return 0;
-	}
-	int i;
-	struct space *space = space_cache_find(def->space_id);
+	uint32_t column_count = def->key_def->part_count;
+	char *aff = (char *)sqlite3DbMallocRaw(db, column_count + 1);
+	if (aff == NULL)
+		return NULL;
+	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 +1939,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));
+		vdbe_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));
+		vdbe_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);

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);
+				vdbe_emit_open_cursor(pParse, cursor,
+						      space->def->id << 10,
+						      space);
 				sqlite3VdbeAddOp2(v, OP_Count, cursor,
 						  sAggInfo.aFunc[0].iMem);
 				sqlite3VdbeAddOp1(v, OP_Close, cursor);

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,
@@ -3580,8 +3564,8 @@ emit_open_cursor(struct Parse *parse, int cursor, int entity_id);
  * @retval address of last opcode.
  */
 int
-sql_emit_open_cursor(struct Parse *parse, int cursor, int index_id,
-		     struct space *space);
+vdbe_emit_open_cursor(struct Parse *parse, int cursor, int index_id,
+		      struct space *space);
 
 int sqlite3ParseUri(const char *, const char *, unsigned int *,
 		    sqlite3_vfs **, char **, char **);
@@ -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);