[tarantool-patches] Re: [PATCH v1 2/2] sql: remove usless sqlite3NestedParse function

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

> Please, finish it. You did not remove Parse.nested and
> PARSE_TAIL. And maybe something else.

Ok, I've removed nested, and some redundant macro definitions.
I've grepp(ed) 'cd src/box/sql; grep -ir nested'
there are still few "nested" words, but they are not about Parser and should be kept.

-----------------------------

From bfb87033cb39d5f5c5c7d36abbbbc80e3b99fd4f Mon Sep 17 00:00:00 2001
Message-Id: <bfb87033cb39d5f5c5c7d36abbbbc80e3b99fd4f.1530900139.git.kshcherbatov@tarantool.org>
In-Reply-To: <cover.1530900139.git.kshcherbatov@tarantool.org>
References: <cover.1530900139.git.kshcherbatov@tarantool.org>
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Date: Wed, 4 Jul 2018 20:05:00 +0300
Subject: [PATCH 2/3] sql: remove usless sqlite3NestedParse function

As sqlite3NestedParse become totaly useless, let's
remove unreacheble code and all mentioning.

Resolves #3496.
---
 src/box/sql/build.c     | 100 +++++-------------------------------------------
 src/box/sql/delete.c    |  13 ++-----
 src/box/sql/insert.c    |  18 +++------
 src/box/sql/parse.y     |  12 +-----
 src/box/sql/sqliteInt.h |  15 ++------
 src/box/sql/tokenize.c  |   2 +-
 src/box/sql/trigger.c   |  45 +++++++---------------
 src/box/sql/update.c    |  15 +++-----
 8 files changed, 45 insertions(+), 175 deletions(-)

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 21791a4..1c00842 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -57,8 +57,6 @@
 void
 sql_finish_coding(struct Parse *parse_context)
 {
-	if (parse_context->nested)
-		return;
 	assert(parse_context->pToplevel == NULL);
 	struct sqlite3 *db = parse_context->db;
 	struct Vdbe *v = sqlite3GetVdbe(parse_context);
@@ -116,46 +114,6 @@ sql_finish_coding(struct Parse *parse_context)
 	}
 }
 
-/*
- * Run the parser and code generator recursively in order to generate
- * code for the SQL statement given onto the end of the pParse context
- * currently under construction.  When the parser is run recursively
- * this way, the final OP_Halt is not appended and other initialization
- * and finalization steps are omitted because those are handling by the
- * outermost parser.
- *
- * Not everything is nestable.  This facility is designed to perform
- * basic DDL operations.  Use care if you decide to try to use this routine
- * for some other purposes.
- */
-void
-sqlite3NestedParse(Parse * pParse, const char *zFormat, ...)
-{
-	va_list ap;
-	char *zSql;
-	char *zErrMsg = 0;
-	sqlite3 *db = pParse->db;
-	char saveBuf[PARSE_TAIL_SZ];
-
-	if (pParse->nErr)
-		return;
-	assert(pParse->nested < 10);	/* Nesting should only be of limited depth */
-	va_start(ap, zFormat);
-	zSql = sqlite3VMPrintf(db, zFormat, ap);
-	va_end(ap);
-	if (zSql == 0) {
-		return;		/* A malloc must have failed */
-	}
-	pParse->nested++;
-	memcpy(saveBuf, PARSE_TAIL(pParse), PARSE_TAIL_SZ);
-	memset(PARSE_TAIL(pParse), 0, PARSE_TAIL_SZ);
-	sqlite3RunParser(pParse, zSql, &zErrMsg);
-	sqlite3DbFree(db, zErrMsg);
-	sqlite3DbFree(db, zSql);
-	memcpy(PARSE_TAIL(pParse), saveBuf, PARSE_TAIL_SZ);
-	pParse->nested--;
-}
-
 /**
  * This is a function which should be called during execution
  * of sqlite3EndTable. It ensures that only PRIMARY KEY
@@ -532,17 +490,10 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
 	Table *pTable;
 	char *zName = 0;	/* The name of the new table */
 	sqlite3 *db = pParse->db;
-	Vdbe *v;
-
-	/* Do not account nested operations: the count of such
-	 * operations depends on Tarantool data dictionary internals,
-	 * such as data layout in system spaces.
-	 */
-	if (!pParse->nested) {
-		if ((v = sqlite3GetVdbe(pParse)) == NULL)
-			goto cleanup;
-		sqlite3VdbeCountChanges(v);
-	}
+	struct Vdbe *v = sqlite3GetVdbe(pParse);
+	if (v == NULL)
+		goto cleanup;
+	sqlite3VdbeCountChanges(v);
 
 	zName = sqlite3NameFromToken(db, pName);
 
@@ -1496,13 +1447,7 @@ createIndex(Parse * pParse, Index * pIndex, int iSpaceId, int iIndexId,
 			  SQL_SUBTYPE_MSGPACK,zParts, P4_STATIC);
 	sqlite3VdbeAddOp3(v, OP_MakeRecord, iFirstCol, 6, iRecord);
 	sqlite3VdbeAddOp2(v, OP_SInsert, BOX_INDEX_ID, iRecord);
-	/* Do not account nested operations: the count of such
-	 * operations depends on Tarantool data dictionary internals,
-	 * such as data layout in system spaces. Also do not account
-	 * autoindexes - they had been accounted as a part of
-	 * CREATE TABLE already.
-	 */
-	if (!pParse->nested && pIndex->idxType == SQLITE_IDXTYPE_APPDEF)
+	if (pIndex->idxType == SQLITE_IDXTYPE_APPDEF)
 		sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
 }
 
@@ -1601,12 +1546,7 @@ createSpace(Parse * pParse, int iSpaceId, char *zStmt)
 			  SQL_SUBTYPE_MSGPACK, zFormat, P4_STATIC);
 	sqlite3VdbeAddOp3(v, OP_MakeRecord, iFirstCol, 7, iRecord);
 	sqlite3VdbeAddOp2(v, OP_SInsert, BOX_SPACE_ID, iRecord);
-	/* Do not account nested operations: the count of such
-	 * operations depends on Tarantool data dictionary internals,
-	 * such as data layout in system spaces.
-	 */
-	if (!pParse->nested)
-		sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
+	sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
 }
 
 /*
@@ -2144,13 +2084,11 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
 	 * Do not account triggers deletion - they will be
 	 * accounted in DELETE from _space below.
 	 */
-	parse_context->nested++;
 	struct sql_trigger *trigger = space->sql_triggers;
 	while (trigger != NULL) {
-		vdbe_code_drop_trigger(parse_context, trigger->zName);
+		vdbe_code_drop_trigger(parse_context, trigger->zName, false);
 		trigger = trigger->next;
 	}
-	parse_context->nested--;
 	/*
 	 * Remove any entries of the _sequence and _space_sequence
 	 * space associated with the table being dropped. This is
@@ -2238,13 +2176,7 @@ sql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list,
 	if (v == NULL || db->mallocFailed) {
 		goto exit_drop_table;
 	}
-	/*
-	 * Activate changes counting here to account
-	 * DROP TABLE IF NOT EXISTS, if the table really does not
-	 * exist.
-	 */
-	if (!parse_context->nested)
-		sqlite3VdbeCountChanges(v);
+	sqlite3VdbeCountChanges(v);
 	assert(parse_context->nErr == 0);
 	assert(table_name_list->nSrc == 1);
 	assert(is_view == 0 || is_view == LOCATE_VIEW);
@@ -2705,13 +2637,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
 	if (db->mallocFailed || parse->nErr > 0) {
 		goto exit_create_index;
 	}
-	/* Do not account nested operations: the count of such
-	 * operations depends on Tarantool data dictionary internals,
-	 * such as data layout in system spaces. Also do not account
-	 * PRIMARY KEY and UNIQUE constraint - they had been accounted
-	 * in CREATE TABLE already.
-	 */
-	if (!parse->nested && idx_type == SQLITE_IDXTYPE_APPDEF) {
+	if (idx_type == SQLITE_IDXTYPE_APPDEF) {
 		Vdbe *v = sqlite3GetVdbe(parse);
 		if (v == NULL)
 			goto exit_create_index;
@@ -3158,13 +3084,7 @@ sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list,
 	if (db->mallocFailed) {
 		goto exit_drop_index;
 	}
-	/*
-	 * Do not account nested operations: the count of such
-	 * operations depends on Tarantool data dictionary internals,
-	 * such as data layout in system spaces.
-	 */
-	if (!parse_context->nested)
-		sqlite3VdbeCountChanges(v);
+	sqlite3VdbeCountChanges(v);
 	assert(index_name_list->nSrc == 1);
 	assert(table_token->n > 0);
 	uint32_t space_id = box_space_id_by_name(table_name,
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 5a79971..66dc0fc 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -156,8 +156,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 	if (v == NULL)
 		goto delete_from_cleanup;
 
-	if (parse->nested == 0)
-		sqlite3VdbeCountChanges(v);
+	sqlite3VdbeCountChanges(v);
 	sql_set_multi_write(parse, true);
 
 	/* If we are trying to delete from a view, realize that
@@ -387,7 +386,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 			idx_noseek = one_pass_cur[1];
 
 		sql_generate_row_delete(parse, table, trigger_list, tab_cursor,
-					reg_key, key_len, parse->nested == 0,
+					reg_key, key_len, true,
 					ON_CONFLICT_ACTION_DEFAULT, one_pass,
 					idx_noseek);
 
@@ -403,13 +402,9 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 		}
 	}
 
-	/* Return the number of rows that were deleted. If this
-	 * routine is generating code because of a call to
-	 * sqlite3NestedParse(), do not invoke the callback
-	 * function.
-	 */
+	/* Return the number of rows that were deleted. */
 	if ((user_session->sql_flags & SQLITE_CountRows) != 0 &&
-	    parse->nested == 0 && parse->pTriggerTab != NULL) {
+	    parse->pTriggerTab != NULL) {
 		sqlite3VdbeAddOp2(v, OP_ResultRow, reg_count, 1);
 		sqlite3VdbeSetNumCols(v, 1);
 		sqlite3VdbeSetColName(v, 0, COLNAME_NAME, "rows deleted",
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index c12043b..58e159c 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -413,13 +413,11 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 		goto insert_cleanup;
 	}
 
-	/* Allocate a VDBE
-	 */
+	/* Allocate a VDBE. */
 	v = sqlite3GetVdbe(pParse);
-	if (v == 0)
+	if (v == NULL)
 		goto insert_cleanup;
-	if (pParse->nested == 0)
-		sqlite3VdbeCountChanges(v);
+	sqlite3VdbeCountChanges(v);
 	sql_set_multi_write(pParse, pSelect != NULL || trigger != NULL);
 
 #ifndef SQLITE_OMIT_XFER_OPT
@@ -904,13 +902,9 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 
  insert_end:
 
-	/*
-	 * Return the number of rows inserted. If this routine is
-	 * generating code because of a call to sqlite3NestedParse(), do not
-	 * invoke the callback function.
-	 */
-	if ((user_session->sql_flags & SQLITE_CountRows) && !pParse->nested
-	    && !pParse->pTriggerTab) {
+	/* Return the number of rows inserted. */
+	if ((user_session->sql_flags & SQLITE_CountRows) != 0 &&
+	    pParse->pTriggerTab == NULL) {
 		sqlite3VdbeAddOp2(v, OP_ResultRow, regRowCount, 1);
 		sqlite3VdbeSetNumCols(v, 1);
 		sqlite3VdbeSetColName(v, 0, COLNAME_NAME, "rows inserted",
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index b2940b7..ac935fd 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -877,18 +877,10 @@ expr(A) ::= VARIABLE(X).     {
     else
         sqlite3ExprAssignVarNumber(pParse, A.pExpr, n);
   }else{
-    /* When doing a nested parse, one can include terms in an expression
-    ** that look like this:   #1 #2 ...  These terms refer to registers
-    ** in the virtual machine.  #N is the N-th register. */
     assert( t.n>=2 );
     spanSet(&A, &t, &t);
-    if( pParse->nested==0 ){
-      sqlite3ErrorMsg(pParse, "near \"%T\": syntax error", &t);
-      A.pExpr = 0;
-    }else{
-      A.pExpr = sqlite3PExpr(pParse, TK_REGISTER, 0, 0);
-      if( A.pExpr ) sqlite3GetInt32(&t.z[1], &A.pExpr->iTable);
-    }
+    sqlite3ErrorMsg(pParse, "near \"%T\": syntax error", &t);
+    A.pExpr = NULL;
   }
 }
 expr(A) ::= expr(A) COLLATE id(C). {
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index d76d173..380c9c6 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2880,7 +2880,6 @@ struct Parse {
 	int rc;			/* Return code from execution */
 	u8 colNamesSet;		/* TRUE after OP_ColumnName has been issued to pVdbe */
 	u8 checkSchema;		/* Causes schema cookie check after an error */
-	u8 nested;		/* Number of nested calls to the parser/code generator */
 	u8 nTempReg;		/* Number of temporary registers in aTempReg[] */
 	u8 isMultiWrite;	/* True if statement may modify/insert multiple rows */
 	u8 mayAbort;		/* True if statement may throw an ABORT exception */
@@ -2976,14 +2975,6 @@ struct Parse {
 };
 
 /*
- * Sizes and pointers of various parts of the Parse object.
- */
-#define PARSE_HDR_SZ offsetof(Parse,aColCache)	/* Recursive part w/o aColCache */
-#define PARSE_RECURSE_SZ offsetof(Parse,sLastToken)	/* Recursive part */
-#define PARSE_TAIL_SZ (sizeof(Parse)-PARSE_RECURSE_SZ)	/* Non-recursive part */
-#define PARSE_TAIL(X) (((char*)(X))+PARSE_RECURSE_SZ)	/* Pointer to tail */
-
-/*
  * Bitfield flags for P5 value in various opcodes.
  *
  * Value constraints (enforced via assert()):
@@ -4129,9 +4120,12 @@ sql_drop_trigger(struct Parse *parser, struct SrcList *name, bool no_err);
  *
  * @param parser Parser context.
  * @param trigger_name The name of trigger to drop.
+ * @param account_changes Increase number of db changes made since
+ *        last reset.
  */
 void
-vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name);
+vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
+		       bool account_changes);
 
 /**
  * Return a list of all triggers on table pTab if there exists at
@@ -4449,7 +4443,6 @@ void sqlite3AlterRenameTable(Parse *, SrcList *, Token *);
 int
 sql_token(const char *z, int *type, bool *is_reserved);
 
-void sqlite3NestedParse(Parse *, const char *, ...);
 void sqlite3ExpirePreparedStatements(sqlite3 *);
 int sqlite3CodeSubselect(Parse *, Expr *, int);
 void sqlite3SelectPrep(Parse *, Select *, NameContext *);
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 7c3dabe..ce9ed84 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -526,7 +526,7 @@ sqlite3RunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
 		pParse->zErrMsg = 0;
 		nErr++;
 	}
-	if (pParse->pVdbe && pParse->nErr > 0 && pParse->nested == 0) {
+	if (pParse->pVdbe != NULL && pParse->nErr > 0) {
 		sqlite3VdbeDelete(pParse->pVdbe);
 		pParse->pVdbe = 0;
 	}
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 801013b..ec0bc98 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -75,16 +75,10 @@ sql_trigger_begin(struct Parse *parse, struct Token *name, int tr_tm,
 	/* The name of the Trigger. */
 	char *trigger_name = NULL;
 
-	/*
-	 * Do not account nested operations: the count of such
-	 * operations depends on Tarantool data dictionary
-	 * internals, such as data layout in system spaces.
-	 */
-	if (!parse->nested) {
-		struct Vdbe *v = sqlite3GetVdbe(parse);
-		if (v != NULL)
-			sqlite3VdbeCountChanges(v);
-	}
+	struct Vdbe *v = sqlite3GetVdbe(parse);
+	if (v != NULL)
+		sqlite3VdbeCountChanges(v);
+
 	/* pName->z might be NULL, but not pName itself. */
 	assert(name != NULL);
 	assert(op == TK_INSERT || op == TK_UPDATE || op == TK_DELETE);
@@ -261,14 +255,8 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep *step_list,
 				  SQL_SUBTYPE_MSGPACK, opts_buff, P4_DYNAMIC);
 		sqlite3VdbeAddOp3(v, OP_MakeRecord, first_col, 3, record);
 		sqlite3VdbeAddOp2(v, OP_IdxInsert, cursor, record);
-		/*
-		 * Do not account nested operations: the count of
-		 * such operations depends on Tarantool data
-		 * dictionary internals, such as data layout in
-		 * system spaces.
-		 */
-		if (!parse->nested)
-			sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
+
+		sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
 		sqlite3VdbeAddOp1(v, OP_Close, cursor);
 
 		sql_set_multi_write(parse, false);
@@ -440,7 +428,8 @@ sql_trigger_delete(struct sqlite3 *db, struct sql_trigger *trigger)
 }
 
 void
-vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name)
+vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
+		       bool account_changes)
 {
 	sqlite3 *db = parser->db;
 	assert(db->pSchema != NULL);
@@ -459,7 +448,7 @@ vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name)
 			  record_to_delete);
 	sqlite3VdbeAddOp2(v, OP_SDelete, BOX_TRIGGER_ID,
 			  record_to_delete);
-	if (parser->nested == 0)
+	if (account_changes)
 		sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
 	sqlite3ChangeCookie(parser);
 }
@@ -473,17 +462,9 @@ sql_drop_trigger(struct Parse *parser, struct SrcList *name, bool no_err)
 		goto drop_trigger_cleanup;
 	assert(db->pSchema != NULL);
 
-	/* Do not account nested operations: the count of such
-	 * operations depends on Tarantool data dictionary internals,
-	 * such as data layout in system spaces. Activate the counter
-	 * here to account DROP TRIGGER IF EXISTS case if the trigger
-	 * actually does not exist.
-	 */
-	if (!parser->nested) {
-		Vdbe *v = sqlite3GetVdbe(parser);
-		if (v != NULL)
-			sqlite3VdbeCountChanges(v);
-	}
+	struct Vdbe *v = sqlite3GetVdbe(parser);
+	if (v != NULL)
+		sqlite3VdbeCountChanges(v);
 
 	assert(name->nSrc == 1);
 	const char *trigger_name = name->a[0].zName;
@@ -495,7 +476,7 @@ sql_drop_trigger(struct Parse *parser, struct SrcList *name, bool no_err)
 					      error_msg, no_err, OP_Found) != 0)
 		goto drop_trigger_cleanup;
 
-	vdbe_code_drop_trigger(parser, trigger_name);
+	vdbe_code_drop_trigger(parser, trigger_name, true);
 
  drop_trigger_cleanup:
 	sqlite3SrcListDelete(db, name);
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 212adbc..cbcca34 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -261,10 +261,9 @@ sqlite3Update(Parse * pParse,		/* The parser context */
 
 	/* Begin generating code. */
 	v = sqlite3GetVdbe(pParse);
-	if (v == 0)
+	if (v == NULL)
 		goto update_cleanup;
-	if (pParse->nested == 0)
-		sqlite3VdbeCountChanges(v);
+	sqlite3VdbeCountChanges(v);
 	sql_set_multi_write(pParse, true);
 
 	/* Allocate required registers. */
@@ -625,13 +624,9 @@ sqlite3Update(Parse * pParse,		/* The parser context */
 	}
 	sqlite3VdbeResolveLabel(v, labelBreak);
 
-	/*
-	 * Return the number of rows that were changed. If this routine is
-	 * generating code because of a call to sqlite3NestedParse(), do not
-	 * invoke the callback function.
-	 */
-	if ((user_session->sql_flags & SQLITE_CountRows) &&
-	    !pParse->pTriggerTab && !pParse->nested) {
+	/* Return the number of rows that were changed. */
+	if (user_session->sql_flags & SQLITE_CountRows &&
+	    pParse->pTriggerTab == NULL) {
 		sqlite3VdbeAddOp2(v, OP_ResultRow, regRowCount, 1);
 		sqlite3VdbeSetNumCols(v, 1);
 		sqlite3VdbeSetColName(v, 0, COLNAME_NAME, "rows updated",
-- 
2.7.4