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 ED95E26364 for ; Tue, 10 Jul 2018 13:08:19 -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 LCuUnOtIAUbI for ; Tue, 10 Jul 2018 13:08:19 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 821002632A for ; Tue, 10 Jul 2018 13:08:19 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v2 3/4] sql: refactor vdbe_emit_open_cursor calls Date: Tue, 10 Jul 2018 20:08:10 +0300 Message-Id: <66f3383d8d49c9ac059a72ff47bf235c9cf11da7.1531242355.git.kshcherbatov@tarantool.org> In-Reply-To: References: In-Reply-To: References: 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: korablev@tarantool.org, Kirill Shcherbatov Made vdbe_emit_open_cursor calls consistent: now it uses index id everywhere. This required to change a way to detect that VDBE has openned Read cursor to specified table in vdbe_has_table_read to write result of insert in temp table if required. --- src/box/sql/analyze.c | 6 +-- src/box/sql/build.c | 3 +- src/box/sql/delete.c | 8 +--- src/box/sql/expr.c | 5 ++- src/box/sql/fkey.c | 3 +- src/box/sql/insert.c | 100 +++++++++++++++++++++++++++++------------------- src/box/sql/pragma.c | 7 +++- src/box/sql/select.c | 4 +- src/box/sql/sqliteInt.h | 6 +-- src/box/sql/vdbe.c | 5 +-- src/box/sql/where.c | 10 ++++- 11 files changed, 92 insertions(+), 65 deletions(-) diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c index 336d146..067a86e 100644 --- a/src/box/sql/analyze.c +++ b/src/box/sql/analyze.c @@ -159,8 +159,7 @@ vdbe_emit_stat_space_open(struct Parse *parse, int stat_cursor, /* Open the sql_stat tables for writing. */ for (uint i = 0; i < lengthof(space_names); ++i) { uint32_t id = space_ids[i]; - int tnum = SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(id, 0); - vdbe_emit_open_cursor(parse, stat_cursor + i, tnum, + vdbe_emit_open_cursor(parse, stat_cursor + i, 0, space_by_id(id)); VdbeComment((v, space_names[i])); } @@ -893,10 +892,11 @@ analyzeOneTable(Parse * pParse, /* Parser context */ /* Open a read-only cursor on the index being analyzed. */ struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum)); + int idx_id = SQLITE_PAGENO_TO_INDEXID(pIdx->tnum); assert(space != NULL); sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space, P4_SPACEPTR); - sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, pIdx->tnum, + sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, idx_id, space_ptr_reg); sql_vdbe_set_p4_key_def(pParse, pIdx); VdbeComment((v, "%s", pIdx->zName)); diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 1c00842..5f7a35a 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2448,7 +2448,8 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage) sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum), 0); struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(tnum)); - vdbe_emit_open_cursor(pParse, iIdx, tnum, space); + vdbe_emit_open_cursor(pParse, iIdx, SQLITE_PAGENO_TO_INDEXID(tnum), + space); sqlite3VdbeChangeP5(v, memRootPage >= 0 ? OPFLAG_P2ISREG : 0); addr1 = sqlite3VdbeAddOp2(v, OP_SorterSort, iSorter, 0); diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c index 66dc0fc..d1c5935 100644 --- a/src/box/sql/delete.c +++ b/src/box/sql/delete.c @@ -340,18 +340,14 @@ 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 = - SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(space_id, - 0); - sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor, - tnum, space_ptr_reg); + sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor, 0, + space_ptr_reg); struct key_def *def = key_def_dup(pk_def); if (def == NULL) { sqlite3OomFault(parse->db); goto delete_from_cleanup; } sqlite3VdbeAppendP4(v, def, P4_KEYDEF); - VdbeComment((v, "%s", space->index[0]->def->name)); if (one_pass == ONEPASS_MULTI) diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 3183e3d..b1650cf 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -2470,8 +2470,11 @@ sqlite3FindInIndex(Parse * pParse, /* Parsing context */ P4_DYNAMIC); struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum)); + uint32_t idx_id = + SQLITE_PAGENO_TO_INDEXID(pIdx-> + tnum); vdbe_emit_open_cursor(pParse, iTab, - pIdx->tnum, space); + idx_id, 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 6c75c47..face9cb 100644 --- a/src/box/sql/fkey.c +++ b/src/box/sql/fkey.c @@ -441,7 +441,8 @@ fkLookupParent(Parse * pParse, /* Parse context */ int regRec = sqlite3GetTempReg(pParse); struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum)); - vdbe_emit_open_cursor(pParse, iCur, pIdx->tnum, space); + uint32_t idx_id = SQLITE_PAGENO_TO_INDEXID(pIdx->tnum); + vdbe_emit_open_cursor(pParse, iCur, idx_id, 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 58e159c..62b85d5 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -179,41 +179,51 @@ sqlite3TableAffinity(Vdbe * v, Table * pTab, int iReg) } } -/* - * Return non-zero if the table pTab in database or any of its indices - * have been opened at any point in the VDBE program. This is used to see if - * a statement of the form "INSERT INTO SELECT ..." can - * run for the results of the SELECT. +/** + * This routine is used to see if a statement of the form + * "INSERT INTO SELECT ..." can run for the results of the + * SELECT. + * + * @param parser Parse context. + * @param table Table AST object. + * @retval true if the table table in database or any of its + * indices have been opened at any point in the VDBE + * program. + * @retval false else. */ -static int -readsTable(Parse * p, Table * pTab) +static bool +vdbe_has_table_read(struct Parse *parser, const struct Table *table) { - Vdbe *v = sqlite3GetVdbe(p); - int i; - int iEnd = sqlite3VdbeCurrentAddr(v); - - for (i = 1; i < iEnd; i++) { - VdbeOp *pOp = sqlite3VdbeGetOp(v, i); - assert(pOp != 0); - /* Currently, there is no difference between - * Read and Write cursors. + struct Vdbe *v = sqlite3GetVdbe(parser); + int last_instr = sqlite3VdbeCurrentAddr(v); + for (int i = 1; i < last_instr; i++) { + struct VdbeOp *op = sqlite3VdbeGetOp(v, i); + assert(op != NULL); + /* + * Currently, there is no difference between Read + * and Write cursors. */ - if (pOp->opcode == OP_OpenRead || - pOp->opcode == OP_OpenWrite) { - Index *pIndex; - int tnum = pOp->p2; - if (tnum == pTab->tnum) { - return 1; - } - for (pIndex = pTab->pIndex; pIndex; - pIndex = pIndex->pNext) { - if (tnum == pIndex->tnum) { - return 1; - } + if (op->opcode == OP_OpenRead || op->opcode == OP_OpenWrite) { + assert(i > 1); + struct VdbeOp *space_var_op = + sqlite3VdbeGetOp(v, i - 1); + assert(space_var_op != NULL); + assert(space_var_op->opcode == OP_LoadPtr); + struct space *space = space_var_op->p4.space; + + if (space->def->id == table->def->id) + return true; + + int idx_id = op->p2; + for (struct Index *pIndex = table->pIndex; + pIndex != NULL; pIndex = pIndex->pNext) { + if (idx_id == + SQLITE_PAGENO_TO_INDEXID(pIndex->tnum)) + return true; } } } - return 0; + return false; } @@ -526,16 +536,20 @@ sqlite3Insert(Parse * pParse, /* Parser context */ assert(pSelect->pEList); nColumn = pSelect->pEList->nExpr; - /* Set useTempTable to TRUE if the result of the SELECT statement - * should be written into a temporary table (template 4). Set to - * FALSE if each output row of the SELECT can be written directly into - * the destination table (template 3). + /* + * Set useTempTable to TRUE if the result of the + * SELECT statement should be written into a + * temporary table (template 4). Set to FALSE if + * each output row of the SELECT can be written + * directly into the destination table + * (template 3). * - * A temp table must be used if the table being updated is also one - * of the tables being read by the SELECT statement. Also use a - * temp table in the case of row triggers. + * A temp table must be used if the table being + * updated is also one of the tables being read by + * the SELECT statement. Also use a temp table in + * the case of row triggers. */ - if (trigger != NULL || readsTable(pParse, pTab)) + if (trigger != NULL || vdbe_has_table_read(pParse, pTab)) useTempTable = 1; if (useTempTable) { @@ -1617,7 +1631,9 @@ sqlite3OpenTableAndIndices(Parse * pParse, /* Parsing context */ p5 = 0; } if (aToOpen == 0 || aToOpen[i + 1]) { - sqlite3VdbeAddOp3(v, op, iIdxCur, pIdx->tnum, + int idx_id = + SQLITE_PAGENO_TO_INDEXID(pIdx->tnum); + sqlite3VdbeAddOp3(v, op, iIdxCur, idx_id, space_ptr_reg); sql_vdbe_set_p4_key_def(pParse, pIdx); sqlite3VdbeChangeP5(v, p5); @@ -1934,11 +1950,15 @@ xferOptimization(Parse * pParse, /* Parser context */ assert(pSrcIdx); struct space *src_space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pSrcIdx->tnum)); - vdbe_emit_open_cursor(pParse, iSrc, pSrcIdx->tnum, src_space); + vdbe_emit_open_cursor(pParse, iSrc, + SQLITE_PAGENO_TO_INDEXID(pSrcIdx->tnum), + src_space); VdbeComment((v, "%s", pSrcIdx->zName)); struct space *dest_space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pDestIdx->tnum)); - vdbe_emit_open_cursor(pParse, iDest, pDestIdx->tnum, dest_space); + vdbe_emit_open_cursor(pParse, iDest, + SQLITE_PAGENO_TO_INDEXID(pDestIdx->tnum), + dest_space); VdbeComment((v, "%s", pDestIdx->zName)); addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iSrc, 0); VdbeCoverage(v); diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index 31581b1..ca119e0 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -685,11 +685,14 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ pParent, OP_OpenRead); } else { + int idx_id = + SQLITE_PAGENO_TO_INDEXID( + pIdx-> + tnum); sqlite3VdbeAddOp3(v, OP_OpenRead, i, - pIdx-> - tnum, + idx_id, 0); sql_vdbe_set_p4_key_def(pParse, pIdx); diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 52b3fdd..ceb7e34 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -6279,9 +6279,7 @@ sqlite3Select(Parse * pParse, /* The parser context */ * Open the cursor, execute the OP_Count, * close the cursor. */ - vdbe_emit_open_cursor(pParse, cursor, - space->def->id << 10, - space); + vdbe_emit_open_cursor(pParse, cursor, 0, 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 380c9c6..4a1eef0 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -3583,9 +3583,9 @@ void sqlite3EndTable(Parse *, Token *, Token *, Select *); * * @param parse_context Parse context. * @param cursor Number of cursor to be created. - * @param index_id Encoded index id (encoding is void actually, so - * pas it as is). In future will be replaced with pointer - * to struct index. + * @param index_id index id. In future will be replaced with + * pointer to struct index. + * @param space Pointer to space object. * @retval address of last opcode. */ int diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 7a4d376..a0eb41b 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3185,9 +3185,8 @@ case OP_ReopenIdx: { pIn3 = &aMem[pOp->p3]; assert(pIn3->flags & MEM_Ptr); if (pCur && pCur->uc.pCursor->space == (struct space *) pIn3->u.p && - pCur->uc.pCursor->index->def->iid == SQLITE_PAGENO_TO_INDEXID(p2)) { + pCur->uc.pCursor->index->def->iid == (uint32_t)p2) goto open_cursor_set_hints; - } /* If the cursor is not currently open or is open on a different * index, then fall through into OP_OpenRead to force a reopen */ @@ -3213,7 +3212,7 @@ case OP_OpenWrite: assert(pIn3->flags & MEM_Ptr); struct space *space = ((struct space *) pIn3->u.p); assert(space != NULL); - struct index *index = space_index(space, SQLITE_PAGENO_TO_INDEXID(p2)); + struct index *index = space_index(space, p2); assert(index != NULL); /* * Since Tarantool iterator provides the full tuple, diff --git a/src/box/sql/where.c b/src/box/sql/where.c index 85143ed..6c07fec 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -4759,10 +4759,16 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */ assert(iIndexCur >= 0); if (op) { if (pIx != NULL) { + uint32_t space_id = + SQLITE_PAGENO_TO_SPACEID(pIx-> + tnum); struct space *space = - space_by_id(SQLITE_PAGENO_TO_SPACEID(pIx->tnum)); + space_by_id(space_id); + uint32_t idx_id = + SQLITE_PAGENO_TO_INDEXID(pIx-> + tnum); vdbe_emit_open_cursor(pParse, iIndexCur, - pIx->tnum, space); + idx_id, space); } else { vdbe_emit_open_cursor(pParse, iIndexCur, idx_def->iid, -- 2.7.4