Re: [Tarantool-patches] [PATCH v2 2/3] sql: don't select from _index during parsing

[Roman Khabibov <roman.habibov@tarantool.org>]

Don’t see the previous mail of this commit.


sql: don't select from _index during parsing

Remove function box_index_by_name() from parser to avoid selects
during parsing. Add the ability to choose index during VDBE code
compilation which will be used to find the tuple to drop from a
system space.

Needed for #4120
---
 src/box/space.h      | 19 +++++++++++
 src/box/sql/build.c  | 79 +++++++++++++++++++++-----------------------
 src/box/sql/pragma.c |  8 ++---
 src/box/sql/vdbe.c   | 10 +++---
 4 files changed, 64 insertions(+), 52 deletions(-)

diff --git a/src/box/space.h b/src/box/space.h
index 9aea4e5be..bbdd3ef70 100644
--- a/src/box/space.h
+++ b/src/box/space.h
@@ -287,6 +287,25 @@ space_index(struct space *space, uint32_t id)
 	return NULL;
 }
 
+/**
+ * Get index by index name.
+ *
+ * @param space Space index belongs to.
+ * @param index_name Name of index to be found.
+ *
+ * @retval NULL if the index is not found.
+ */
+static inline struct index *
+space_index_by_name(struct space *space, const char *index_name)
+{
+	for(uint32_t i = 0; i < space->index_count; i++) {
+		struct index *index = space->index[i];
+		if (strcmp(index_name, index->def->name) == 0)
+			return index;
+	}
+	return NULL;
+}
+
 /**
  * Return true if the unique constraint must be checked for
  * the index with the given id before inserting a tuple into
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index d9bf8de91..fef069640 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -47,7 +47,6 @@
 #include "sqlInt.h"
 #include "vdbeInt.h"
 #include "tarantoolInt.h"
-#include "box/box.h"
 #include "box/ck_constraint.h"
 #include "box/fk_constraint.h"
 #include "box/sequence.h"
@@ -116,24 +115,6 @@ sql_finish_coding(struct Parse *parse_context)
 		parse_context->is_aborted = true;
 	}
 }
-/**
- * Find index by its name.
- *
- * @param space Space index belongs to.
- * @param name Name of index to be found.
- *
- * @retval NULL in case index doesn't exist.
- */
-static struct index *
-sql_space_index_by_name(struct space *space, const char *name)
-{
-	for (uint32_t i = 0; i < space->index_count; ++i) {
-		struct index *idx = space->index[i];
-		if (strcmp(name, idx->def->name) == 0)
-			return idx;
-	}
-	return NULL;
-}
 
 bool
 sql_space_column_is_in_pk(struct space *space, uint32_t column)
@@ -1469,6 +1450,40 @@ vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
 	sql_table_delete_from(parse, src_list, where);
 }
 
+/**
+ * Generate VDBE program to remove entry from _index space.
+ *
+ * @param parse_context Parsing context.
+ * @param name Index name.
+ * @param space_def Def of table which index belongs to.
+ * @param errcode Type of printing error: "no such index" or
+ *                "no such constraint".
+ * @param if_exist True if there was <IF EXISTS> in the query.
+ */
+static void
+vdbe_emit_index_drop(struct Parse *parse_context, const char *name,
+		     struct space_def *space_def, int errcode, bool if_exist)
+{
+	assert(errcode == ER_NO_SUCH_INDEX_NAME ||
+	       errcode == ER_NO_SUCH_CONSTRAINT);
+	struct Vdbe *vdbe = sqlGetVdbe(parse_context);
+	assert(vdbe != NULL);
+	assert(parse_context->db != NULL);
+	int key_reg = sqlGetTempRange(parse_context, 3);
+	sqlVdbeAddOp2(vdbe, OP_Integer, space_def->id, key_reg);
+	sqlVdbeAddOp4(vdbe, OP_String8, 0, key_reg + 1, 0,
+		      sqlDbStrDup(parse_context->db, name), P4_DYNAMIC);
+	const char *error_msg =
+		tt_sprintf(tnt_errcode_desc(errcode), name, space_def->name);
+	if (vdbe_emit_halt_with_presence_test(parse_context, BOX_INDEX_ID, 2,
+					      key_reg, 2, errcode, error_msg,
+					      if_exist, OP_Found) != 0)
+		return;
+	sqlVdbeAddOp3(vdbe, OP_MakeRecord, key_reg, 2, key_reg + 2);
+	sqlVdbeAddOp3(vdbe, OP_SDelete, BOX_INDEX_ID, key_reg + 2, 2);
+	sqlReleaseTempRange(parse_context, key_reg, 3);
+}
+
 /**
  * Generate VDBE program to remove entry from _fk_constraint space.
  *
@@ -2401,7 +2416,7 @@ sql_create_index(struct Parse *parse) {
 			parse->is_aborted = true;
 			goto exit_create_index;
 		}
-		if (sql_space_index_by_name(space, name) != NULL) {
+		if (space_index_by_name(space, name) != NULL) {
 			if (! create_entity_def->if_not_exist) {
 				diag_set(ClientError, ER_INDEX_EXISTS_IN_SPACE,
 					 name, def->name);
@@ -2695,29 +2710,9 @@ sql_drop_index(struct Parse *parse_context)
 		parse_context->is_aborted = true;
 		goto exit_drop_index;
 	}
-	uint32_t index_id = box_index_id_by_name(space->def->id, index_name,
-						 strlen(index_name));
-	if (index_id == BOX_ID_NIL) {
-		if (!if_exists) {
-			diag_set(ClientError, ER_NO_SUCH_INDEX_NAME,
-				 index_name, table_name);
-			parse_context->is_aborted = true;
-		}
-		goto exit_drop_index;
-	}
 
-	/*
-	 * Generate code to remove entry from _index space
-	 * But firstly, delete statistics since schema
-	 * changes after DDL.
-	 */
-	int record_reg = ++parse_context->nMem;
-	int space_id_reg = ++parse_context->nMem;
-	int index_id_reg = ++parse_context->nMem;
-	sqlVdbeAddOp2(v, OP_Integer, space->def->id, space_id_reg);
-	sqlVdbeAddOp2(v, OP_Integer, index_id, index_id_reg);
-	sqlVdbeAddOp3(v, OP_MakeRecord, space_id_reg, 2, record_reg);
-	sqlVdbeAddOp2(v, OP_SDelete, BOX_INDEX_ID, record_reg);
+	vdbe_emit_index_drop(parse_context, index_name, space->def,
+			     ER_NO_SUCH_INDEX_NAME, if_exists);
 	sqlVdbeChangeP5(v, OPFLAG_NCHANGE);
  exit_drop_index:
 	sqlSrcListDelete(db, table_list);
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index a77bf4b16..c15f2e0d1 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -33,7 +33,6 @@
  * This file contains code used to implement the PRAGMA command.
  */
 #include "box/index.h"
-#include "box/box.h"
 #include "box/tuple.h"
 #include "box/fk_constraint.h"
 #include "box/schema.h"
@@ -192,12 +191,9 @@ sql_pragma_index_info(struct Parse *parse,
 	struct space *space = space_by_name(tbl_name);
 	if (space == NULL)
 		return;
-	uint32_t iid = box_index_id_by_name(space->def->id, idx_name,
-					    strlen(idx_name));
-	if (iid == BOX_ID_NIL)
+	struct index *idx = space_index_by_name(space, idx_name);
+	if (idx == NULL)
 		return;
-	struct index *idx = space_index(space, iid);
-	assert(idx != NULL);
 	parse->nMem = 6;
 	struct Vdbe *v = sqlGetVdbe(parse);
 	assert(v != NULL);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 620d74e66..912a10153 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4460,11 +4460,12 @@ case OP_SInsert: {
 	break;
 }
 
-/* Opcode: SDelete P1 P2 * * P5
- * Synopsis: space id = P1, key = r[P2]
+/* Opcode: SDelete P1 P2 P3 * P5
+ * Synopsis: space id = P1, key = r[P2], searching index id = P3
  *
  * This opcode is used only during DDL routine.
- * Delete entry with given key from system space.
+ * Delete entry with given key from system space. P3 is the index
+ * number by which to search for the key.
  *
  * If P5 is set to OPFLAG_NCHANGE, account overall changes
  * made to database.
@@ -4472,13 +4473,14 @@ case OP_SInsert: {
 case OP_SDelete: {
 	assert(pOp->p1 > 0);
 	assert(pOp->p2 >= 0);
+	assert(pOp->p3 >= 0);
 
 	pIn2 = &aMem[pOp->p2];
 	struct space *space = space_by_id(pOp->p1);
 	assert(space != NULL);
 	assert(space_is_system(space));
 	assert(p->errorAction == ON_CONFLICT_ACTION_ABORT);
-	if (sql_delete_by_key(space, 0, pIn2->z, pIn2->n) != 0)
+	if (sql_delete_by_key(space, pOp->p3, pIn2->z, pIn2->n) != 0)
 		goto abort_due_to_error;
 	if (pOp->p5 & OPFLAG_NCHANGE)
 		p->nChange++;
-- 
2.21.0 (Apple Git-122)