From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 86F8224A61 for ; Fri, 6 Jul 2018 14:13:24 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fcO8biwYtj0E for ; Fri, 6 Jul 2018 14:13:24 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id A73AC24A4F for ; Fri, 6 Jul 2018 14:13:23 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 2/2] sql: remove usless sqlite3NestedParse function References: <444b91093cc6ea1648f0e8bdd9b0923694ca3f8a.1530724375.git.kshcherbatov@tarantool.org> From: Kirill Shcherbatov Message-ID: <2facdb81-0f02-5d1b-fd6b-7b9ea1a436ec@tarantool.org> Date: Fri, 6 Jul 2018 21:13:19 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy > 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: In-Reply-To: References: From: Kirill Shcherbatov 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