* [tarantool-patches] [PATCH v1 1/1] sql: drop useless legacy Cursor hints @ 2018-06-09 13:09 Kirill Shcherbatov 2018-06-09 21:41 ` [tarantool-patches] " Vladislav Shpilevoy 0 siblings, 1 reply; 11+ messages in thread From: Kirill Shcherbatov @ 2018-06-09 13:09 UTC (permalink / raw) To: tarantool-patches; +Cc: v.shpilevoy, Kirill Shcherbatov Deleted BTREE_BULKLOAD and OPFLAG_BULKCSR as there become useless since btree.c was dropped as a part of #2419. Removed SQLITE_ENABLE_CURSOR_HINTS sections as this code has never been in use. Start use OPFLAG_SEEKEQ instead of equal BTREE_SEEK_EQ as this macro name has no sense since no btree present in SQL. Resolves #3121. --- src/box/sql.c | 4 +- src/box/sql/build.c | 4 +- src/box/sql/cursor.c | 2 +- src/box/sql/cursor.h | 16 ---- src/box/sql/expr.c | 18 ---- src/box/sql/insert.c | 1 - src/box/sql/sqliteInt.h | 6 -- src/box/sql/vdbe.c | 19 ++-- src/box/sql/vdbe.h | 3 - src/box/sql/vdbeaux.c | 133 --------------------------- src/box/sql/where.c | 6 -- src/box/sql/wherecode.c | 234 ------------------------------------------------ 12 files changed, 11 insertions(+), 435 deletions(-) diff --git a/src/box/sql.c b/src/box/sql.c index 7379cb4..e644056 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -292,12 +292,12 @@ int tarantoolSqlite3MovetoUnpacked(BtCursor *pCur, UnpackedRecord *pIdxKey, res_success = -1; /* item<key */ break; case OP_SeekLE: - pCur->iter_type = (pCur->hints & BTREE_SEEK_EQ) ? + pCur->iter_type = (pCur->hints & OPFLAG_SEEKEQ) ? ITER_REQ : ITER_LE; res_success = 0; /* item==key */ break; case OP_SeekGE: - pCur->iter_type = (pCur->hints & BTREE_SEEK_EQ) ? + pCur->iter_type = (pCur->hints & OPFLAG_SEEKEQ) ? ITER_EQ : ITER_GE; res_success = 0; /* item==key */ break; diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 28e4d7a..93bf0cc 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2676,9 +2676,7 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage) sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum), 0); emit_open_cursor(pParse, iIdx, tnum); - sqlite3VdbeChangeP5(v, - OPFLAG_BULKCSR | ((memRootPage >= 0) ? - OPFLAG_P2ISREG : 0)); + sqlite3VdbeChangeP5(v, memRootPage >= 0 ? OPFLAG_P2ISREG : 0); addr1 = sqlite3VdbeAddOp2(v, OP_SorterSort, iSorter, 0); VdbeCoverage(v); diff --git a/src/box/sql/cursor.c b/src/box/sql/cursor.c index 82ad08b..9dd2b02 100644 --- a/src/box/sql/cursor.c +++ b/src/box/sql/cursor.c @@ -53,7 +53,7 @@ sql_cursor_cleanup(struct BtCursor *cursor) void sqlite3CursorHintFlags(BtCursor * pCur, unsigned x) { - assert(x == BTREE_SEEK_EQ || x == BTREE_BULKLOAD || x == 0); + assert(x == OPFLAG_SEEKEQ || x == 0); pCur->hints = x; } diff --git a/src/box/sql/cursor.h b/src/box/sql/cursor.h index c559417..252aaac 100644 --- a/src/box/sql/cursor.h +++ b/src/box/sql/cursor.h @@ -35,22 +35,6 @@ typedef struct BtCursor BtCursor; /* - * Values that may be OR'd together to form the argument to the - * BTREE_HINT_FLAGS hint for sqlite3BtreeCursorHint(): - * - * The BTREE_BULKLOAD flag is set on index cursors when the index is going - * to be filled with content that is already in sorted order. - * - * The BTREE_SEEK_EQ flag is set on cursors that will get OP_SeekGE or - * OP_SeekLE opcodes for a range search, but where the range of entries - * selected will all have the same key. In other words, the cursor will - * be used only for equality key searches. - * - */ -#define BTREE_BULKLOAD 0x00000001 /* Used to full index in sorted order */ -#define BTREE_SEEK_EQ 0x00000002 /* EQ seeks only - no range seeks */ - -/* * A cursor contains a particular entry either from Tarantrool or * Sorter. Tarantool cursor is able to point to ordinary table or * ephemeral one. To distinguish them curFlags is set to TaCursor diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 8866f6f..371d5d5 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -2000,24 +2000,6 @@ sqlite3ExprIsConstantOrFunction(Expr * p, u8 isInit) return exprIsConst(p, 4 + isInit, 0); } -#ifdef SQLITE_ENABLE_CURSOR_HINTS -/* - * Walk an expression tree. Return 1 if the expression contains a - * subquery of some kind. Return 0 if there are no subqueries. - */ -int -sqlite3ExprContainsSubquery(Expr * p) -{ - Walker w; - memset(&w, 0, sizeof(w)); - w.eCode = 1; - w.xExprCallback = sqlite3ExprWalkNoop; - w.xSelectCallback = selectNodeIsConstant; - sqlite3WalkExpr(&w, p); - return w.eCode == 0; -} -#endif - /* * If the expression p codes a constant integer that is small enough * to fit in a 32-bit integer, return 1 and put the value of the integer diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 59c61c7..a3a5b45 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -1963,7 +1963,6 @@ xferOptimization(Parse * pParse, /* Parser context */ VdbeComment((v, "%s", pSrcIdx->zName)); emit_open_cursor(pParse, iDest, pDestIdx->tnum); sql_vdbe_set_p4_key_def(pParse, pDestIdx); - sqlite3VdbeChangeP5(v, OPFLAG_BULKCSR); VdbeComment((v, "%s", pDestIdx->zName)); addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iSrc, 0); VdbeCoverage(v); diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 01351a1..c95b1f0 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -2978,8 +2978,6 @@ struct Parse { * Value constraints (enforced via assert()): * OPFLAG_LENGTHARG == SQLITE_FUNC_LENGTH * OPFLAG_TYPEOFARG == SQLITE_FUNC_TYPEOF - * OPFLAG_BULKCSR == BTREE_BULKLOAD - * OPFLAG_SEEKEQ == BTREE_SEEK_EQ * OPFLAG_FORDELETE == BTREE_FORDELETE * OPFLAG_SAVEPOSITION == BTREE_SAVEPOSITION * OPFLAG_AUXDELETE == BTREE_AUXDELETE @@ -2995,7 +2993,6 @@ struct Parse { #endif #define OPFLAG_LENGTHARG 0x40 /* OP_Column only used for length() */ #define OPFLAG_TYPEOFARG 0x80 /* OP_Column only used for typeof() */ -#define OPFLAG_BULKCSR 0x01 /* OP_Open** used to open bulk cursor */ #define OPFLAG_SEEKEQ 0x02 /* OP_Open** cursor uses EQ seek only */ #define OPFLAG_FORDELETE 0x08 /* OP_Open should use BTREE_FORDELETE */ #define OPFLAG_P2ISREG 0x10 /* P2 to OP_Open** is a register number */ @@ -3834,9 +3831,6 @@ int sqlite3ExprIsConstant(Expr *); int sqlite3ExprIsConstantNotJoin(Expr *); int sqlite3ExprIsConstantOrFunction(Expr *, u8); int sqlite3ExprIsTableConstant(Expr *, int); -#ifdef SQLITE_ENABLE_CURSOR_HINTS -int sqlite3ExprContainsSubquery(Expr *); -#endif int sqlite3ExprIsInteger(Expr *, int *); int sqlite3ExprCanBeNull(const Expr *); int sqlite3ExprNeedsNoAffinityChange(const Expr *, char); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 3fe5875..3139933 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3206,14 +3206,7 @@ case OP_OpenWrite: pCur->key_def = index->def->key_def; open_cursor_set_hints: - assert(OPFLAG_BULKCSR==BTREE_BULKLOAD); - assert(OPFLAG_SEEKEQ==BTREE_SEEK_EQ); - testcase( pOp->p5 & OPFLAG_BULKCSR); -#ifdef SQLITE_ENABLE_CURSOR_HINTS - testcase( pOp->p2 & OPFLAG_SEEKEQ); -#endif - sqlite3CursorHintFlags(pCur->uc.pCursor, - (pOp->p5 & (OPFLAG_BULKCSR|OPFLAG_SEEKEQ))); + sqlite3CursorHintFlags(pCur->uc.pCursor, pOp->p5 & OPFLAG_SEEKEQ); if (rc) goto abort_due_to_error; break; } @@ -3527,11 +3520,13 @@ case OP_SeekGT: { /* jump, in3 */ } } } - /* For a cursor with the BTREE_SEEK_EQ hint, only the OP_SeekGE and - * OP_SeekLE opcodes are allowed, and these must be immediately followed - * by an OP_IdxGT or OP_IdxLT opcode, respectively, with the same key. + /* + * For a cursor with the OPFLAG_SEEKEQ hint, only the + * OP_SeekGE and OP_SeekLE opcodes are allowed, and these + * must be immediately followed by an OP_IdxGT or + * OP_IdxLT opcode, respectively, with the same key. */ - if (sqlite3CursorHasHint(pC->uc.pCursor, BTREE_SEEK_EQ)) { + if (sqlite3CursorHasHint(pC->uc.pCursor, OPFLAG_SEEKEQ) != 0) { eqOnly = 1; assert(pOp->opcode==OP_SeekGE || pOp->opcode==OP_SeekLE); assert(pOp[1].opcode==OP_IdxLT || pOp[1].opcode==OP_IdxGT); diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h index 68d542c..4dcc457 100644 --- a/src/box/sql/vdbe.h +++ b/src/box/sql/vdbe.h @@ -80,9 +80,6 @@ struct VdbeOp { int *ai; /* Used when p4type is P4_INTARRAY */ SubProgram *pProgram; /* Used when p4type is P4_SUBPROGRAM */ Index *pIndex; /* Used when p4type is P4_INDEX */ -#ifdef SQLITE_ENABLE_CURSOR_HINTS - Expr *pExpr; /* Used when p4type is P4_EXPR */ -#endif int (*xAdvance) (BtCursor *, int *); /** Used when p4type is P4_KEYDEF. */ struct key_def *key_def; diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 679bd0b..a29d0a3 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -972,12 +972,6 @@ freeP4(sqlite3 * db, int p4type, void *p4) case P4_KEYDEF: key_def_delete(p4); break; -#ifdef SQLITE_ENABLE_CURSOR_HINTS - case P4_EXPR:{ - sqlite3ExprDelete(db, (Expr *) p4); - break; - } -#endif case P4_FUNCDEF:{ freeEphemeralFunction(db, (FuncDef *) p4); break; @@ -1389,127 +1383,6 @@ displayComment(const Op * pOp, /* The opcode to be commented */ } #endif /* SQLITE_DEBUG */ -#if defined(SQLITE_ENABLE_CURSOR_HINTS) -/* - * Translate the P4.pExpr value for an OP_CursorHint opcode into text - * that can be displayed in the P4 column of EXPLAIN output. - */ -static void -displayP4Expr(StrAccum * p, Expr * pExpr) -{ - const char *zOp = 0; - switch (pExpr->op) { - case TK_STRING: - sqlite3XPrintf(p, "%Q", pExpr->u.zToken); - break; - case TK_INTEGER: - sqlite3XPrintf(p, "%d", pExpr->u.iValue); - break; - case TK_NULL: - sqlite3XPrintf(p, "NULL"); - break; - case TK_REGISTER:{ - sqlite3XPrintf(p, "r[%d]", pExpr->iTable); - break; - } - case TK_COLUMN:{ - if (pExpr->iColumn < 0) { - sqlite3XPrintf(p, "rowid"); - } else { - sqlite3XPrintf(p, "c%d", (int)pExpr->iColumn); - } - break; - } - case TK_LT: - zOp = "LT"; - break; - case TK_LE: - zOp = "LE"; - break; - case TK_GT: - zOp = "GT"; - break; - case TK_GE: - zOp = "GE"; - break; - case TK_NE: - zOp = "NE"; - break; - case TK_EQ: - zOp = "EQ"; - break; - case TK_AND: - zOp = "AND"; - break; - case TK_OR: - zOp = "OR"; - break; - case TK_PLUS: - zOp = "ADD"; - break; - case TK_STAR: - zOp = "MUL"; - break; - case TK_MINUS: - zOp = "SUB"; - break; - case TK_REM: - zOp = "REM"; - break; - case TK_BITAND: - zOp = "BITAND"; - break; - case TK_BITOR: - zOp = "BITOR"; - break; - case TK_SLASH: - zOp = "DIV"; - break; - case TK_LSHIFT: - zOp = "LSHIFT"; - break; - case TK_RSHIFT: - zOp = "RSHIFT"; - break; - case TK_CONCAT: - zOp = "CONCAT"; - break; - case TK_UMINUS: - zOp = "MINUS"; - break; - case TK_UPLUS: - zOp = "PLUS"; - break; - case TK_BITNOT: - zOp = "BITNOT"; - break; - case TK_NOT: - zOp = "NOT"; - break; - case TK_ISNULL: - zOp = "IS NULL"; - break; - case TK_NOTNULL: - zOp = "NOT NULL"; - break; - - default: - sqlite3XPrintf(p, "%s", "expr"); - break; - } - - if (zOp) { - sqlite3XPrintf(p, "%s(", zOp); - displayP4Expr(p, pExpr->pLeft); - if (pExpr->pRight) { - sqlite3StrAccumAppend(p, ",", 1); - displayP4Expr(p, pExpr->pRight); - } - sqlite3StrAccumAppend(p, ")", 1); - } -} -#endif /* defined(SQLITE_ENABLE_CURSOR_HINTS) */ - /* * Compute a string that describes the P4 parameter for an opcode. * Use zTemp for any required temporary buffer space. @@ -1549,12 +1422,6 @@ displayP4(Op * pOp, char *zTemp, int nTemp) } break; } -#ifdef SQLITE_ENABLE_CURSOR_HINTS - case P4_EXPR:{ - displayP4Expr(&x, pOp->p4.pExpr); - break; - } -#endif case P4_COLLSEQ:{ struct coll *pColl = pOp->p4.pColl; if (pColl != NULL) diff --git a/src/box/sql/where.c b/src/box/sql/where.c index e791647..1ca366c 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -4588,12 +4588,6 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */ && pTab->nCol == BMS - 1); testcase(pWInfo->eOnePass == ONEPASS_OFF && pTab->nCol == BMS); -#ifdef SQLITE_ENABLE_CURSOR_HINTS - if (pLoop->pIndex != 0) { - sqlite3VdbeChangeP5(v, - OPFLAG_SEEKEQ | bFordelete); - } else -#endif { sqlite3VdbeChangeP5(v, bFordelete); } diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c index 09b2671..26eca21 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -817,241 +817,7 @@ whereLikeOptimizationStringFixup(Vdbe * v, /* prepared statement under construc #define whereLikeOptimizationStringFixup(A,B,C) #endif -#ifdef SQLITE_ENABLE_CURSOR_HINTS -/* - * Information is passed from codeCursorHint() down to individual nodes of - * the expression tree (by sqlite3WalkExpr()) using an instance of this - * structure. - */ -struct CCurHint { - int iTabCur; /* Cursor for the main table */ - int iIdxCur; /* Cursor for the index, if pIdx!=0. Unused otherwise */ - Index *pIdx; /* The index used to access the table */ -}; - -/* - * This function is called for every node of an expression that is a candidate - * for a cursor hint on an index cursor. For TK_COLUMN nodes that reference - * the table CCurHint.iTabCur, verify that the same column can be - * accessed through the index. If it cannot, then set pWalker->eCode to 1. - */ -static int -codeCursorHintCheckExpr(Walker * pWalker, Expr * pExpr) -{ - struct CCurHint *pHint = pWalker->u.pCCurHint; - assert(pHint->pIdx != 0); - if (pExpr->op == TK_COLUMN - && pExpr->iTable == pHint->iTabCur - && (pExpr->iColumn < 0)) { - pWalker->eCode = 1; - } - return WRC_Continue; -} - -/* - * Test whether or not expression pExpr, which was part of a WHERE clause, - * should be included in the cursor-hint for a table that is on the rhs - * of a LEFT JOIN. Set Walker.eCode to non-zero before returning if the - * expression is not suitable. - * - * An expression is unsuitable if it might evaluate to non NULL even if - * a TK_COLUMN node that does affect the value of the expression is set - * to NULL. For example: - * - * col IS NULL - * col IS NOT NULL - * coalesce(col, 1) - * CASE WHEN col THEN 0 ELSE 1 END - */ -static int -codeCursorHintIsOrFunction(Walker * pWalker, Expr * pExpr) -{ - if (pExpr->op == TK_ISNULL - || pExpr->op == TK_NOTNULL || pExpr->op == TK_CASE) { - pWalker->eCode = 1; - } else if (pExpr->op == TK_FUNCTION) { - int d1; - char d2[3]; - if (0 == - sqlite3IsLikeFunction(pWalker->pParse->db, pExpr, &d1, - d2)) { - pWalker->eCode = 1; - } - } - - return WRC_Continue; -} - -/* - * This function is called on every node of an expression tree used as an - * argument to the OP_CursorHint instruction. If the node is a TK_COLUMN - * that accesses any table other than the one identified by - * CCurHint.iTabCur, then do the following: - * - * 1) allocate a register and code an OP_Column instruction to read - * the specified column into the new register, and - * - * 2) transform the expression node to a TK_REGISTER node that reads - * from the newly populated register. - * - * Also, if the node is a TK_COLUMN that does access the table idenified - * by pCCurHint.iTabCur, and an index is being used (which we will - * know because CCurHint.pIdx!=0) then transform the TK_COLUMN into - * an access of the index rather than the original table. - */ -static int -codeCursorHintFixExpr(Walker * pWalker, Expr * pExpr) -{ - int rc = WRC_Continue; - struct CCurHint *pHint = pWalker->u.pCCurHint; - if (pExpr->op == TK_COLUMN) { - if (pExpr->iTable != pHint->iTabCur) { - Vdbe *v = pWalker->pParse->pVdbe; - int reg = ++pWalker->pParse->nMem; /* Register for column value */ - sqlite3ExprCodeGetColumnOfTable(v, pExpr->pTab->def, - pExpr->iTable, - pExpr->iColumn, reg); - pExpr->op = TK_REGISTER; - pExpr->iTable = reg; - } else if (pHint->pIdx != 0) { - pExpr->iTable = pHint->iIdxCur; - assert(pExpr->iColumn >= 0); - } - } else if (pExpr->op == TK_AGG_FUNCTION) { - /* An aggregate function in the WHERE clause of a query means this must - * be a correlated sub-query, and expression pExpr is an aggregate from - * the parent context. Do not walk the function arguments in this case. - * - * todo: It should be possible to replace this node with a TK_REGISTER - * expression, as the result of the expression must be stored in a - * register at this point. The same holds for TK_AGG_COLUMN nodes. - */ - rc = WRC_Prune; - } - return rc; -} - -/* - * Insert an OP_CursorHint instruction if it is appropriate to do so. - */ -static void -codeCursorHint(struct SrcList_item *pTabItem, /* FROM clause item */ - WhereInfo * pWInfo, /* The where clause */ - WhereLevel * pLevel, /* Which loop to provide hints for */ - WhereTerm * pEndRange) /* Hint this end-of-scan boundary term if not NULL */ -{ - Parse *pParse = pWInfo->pParse; - sqlite3 *db = pParse->db; - Vdbe *v = pParse->pVdbe; - Expr *pExpr = 0; - WhereLoop *pLoop = pLevel->pWLoop; - int iCur; - WhereClause *pWC; - WhereTerm *pTerm; - int i, j; - struct CCurHint sHint; - Walker sWalker; - - if (OptimizationDisabled(db, SQLITE_CursorHints)) - return; - iCur = pLevel->iTabCur; - assert(iCur == pWInfo->pTabList->a[pLevel->iFrom].iCursor); - sHint.iTabCur = iCur; - sHint.iIdxCur = pLevel->iIdxCur; - sHint.pIdx = pLoop->pIndex; - memset(&sWalker, 0, sizeof(sWalker)); - sWalker.pParse = pParse; - sWalker.u.pCCurHint = &sHint; - pWC = &pWInfo->sWC; - for (i = 0; i < pWC->nTerm; i++) { - pTerm = &pWC->a[i]; - if (pTerm->wtFlags & (TERM_VIRTUAL | TERM_CODED)) - continue; - if (pTerm->prereqAll & pLevel->notReady) - continue; - - /* Any terms specified as part of the ON(...) clause for any LEFT - * JOIN for which the current table is not the rhs are omitted - * from the cursor-hint. - * - * If this table is the rhs of a LEFT JOIN, "IS" or "IS NULL" terms - * that were specified as part of the WHERE clause must be excluded. - * This is to address the following: - * - * SELECT ... t1 LEFT JOIN t2 ON (t1.a=t2.b) WHERE t2.c IS NULL; - * - * Say there is a single row in t2 that matches (t1.a=t2.b), but its - * t2.c values is not NULL. If the (t2.c IS NULL) constraint is - * pushed down to the cursor, this row is filtered out, causing - * SQLite to synthesize a row of NULL values. Which does match the - * WHERE clause, and so the query returns a row. Which is incorrect. - * - * For the same reason, WHERE terms such as: - * - * WHERE 1 = (t2.c IS NULL) - * - * are also excluded. See codeCursorHintIsOrFunction() for details. - */ - if (pTabItem->fg.jointype & JT_LEFT) { - Expr *pExpr = pTerm->pExpr; - if (!ExprHasProperty(pExpr, EP_FromJoin) - || pExpr->iRightJoinTable != pTabItem->iCursor) { - sWalker.eCode = 0; - sWalker.xExprCallback = - codeCursorHintIsOrFunction; - sqlite3WalkExpr(&sWalker, pTerm->pExpr); - if (sWalker.eCode) - continue; - } - } else { - if (ExprHasProperty(pTerm->pExpr, EP_FromJoin)) - continue; - } - - /* All terms in pWLoop->aLTerm[] except pEndRange are used to initialize - * the cursor. These terms are not needed as hints for a pure range - * scan (that has no == terms) so omit them. - */ - if (pLoop->nEq == 0 && pTerm != pEndRange) { - for (j = 0; - j < pLoop->nLTerm && pLoop->aLTerm[j] != pTerm; - j++) { - } - if (j < pLoop->nLTerm) - continue; - } - - /* No subqueries or non-deterministic functions allowed */ - if (sqlite3ExprContainsSubquery(pTerm->pExpr)) - continue; - - /* For an index scan, make sure referenced columns are actually in - * the index. - */ - if (sHint.pIdx != 0) { - sWalker.eCode = 0; - sWalker.xExprCallback = codeCursorHintCheckExpr; - sqlite3WalkExpr(&sWalker, pTerm->pExpr); - if (sWalker.eCode) - continue; - } - - /* If we survive all prior tests, that means this term is worth hinting */ - pExpr = - sqlite3ExprAnd(db, pExpr, - sqlite3ExprDup(db, pTerm->pExpr, 0)); - } - if (pExpr != 0) { - sWalker.xExprCallback = codeCursorHintFixExpr; - sqlite3WalkExpr(&sWalker, pExpr); - sqlite3VdbeAddOp4(v, OP_CursorHint, - (sHint.pIdx ? sHint.iIdxCur : sHint.iTabCur), - 0, 0, (const char *)pExpr, P4_EXPR); - } -} -#else #define codeCursorHint(A,B,C,D) /* No-op */ -#endif /* SQLITE_ENABLE_CURSOR_HINTS */ /* * If the expression passed as the second argument is a vector, generate -- 2.7.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [tarantool-patches] [PATCH v1 1/1] sql: drop useless legacy Cursor hints 2018-06-09 13:09 [tarantool-patches] [PATCH v1 1/1] sql: drop useless legacy Cursor hints Kirill Shcherbatov @ 2018-06-09 21:41 ` Vladislav Shpilevoy 2018-06-13 6:43 ` [tarantool-patches] " Kirill Shcherbatov 0 siblings, 1 reply; 11+ messages in thread From: Vladislav Shpilevoy @ 2018-06-09 21:41 UTC (permalink / raw) To: tarantool-patches; +Cc: Kirill Shcherbatov Hello. Thanks for the patch! > >Deleted BTREE_BULKLOAD and OPFLAG_BULKCSR as there become >useless since btree.c was dropped as a part of #2419. >Removed SQLITE_ENABLE_CURSOR_HINTS sections as this code >has never been in use. >Start use OPFLAG_SEEKEQ instead of equal BTREE_SEEK_EQ as >this macro name has no sense since no btree present in SQL. > >Resolves #3121. >--- 1. The same comment. What have you write after '---'? 2. I have pushed my fixes as a separate commit. Please, look, perceive and squash. > src/box/sql.c | 4 +- > src/box/sql/build.c | 4 +- > src/box/sql/cursor.c | 2 +- > src/box/sql/cursor.h | 16 ---- > src/box/sql/expr.c | 18 ---- > src/box/sql/insert.c | 1 - > src/box/sql/sqliteInt.h | 6 -- > src/box/sql/vdbe.c | 19 ++-- > src/box/sql/vdbe.h | 3 - > src/box/sql/vdbeaux.c | 133 --------------------------- > src/box/sql/where.c | 6 -- > src/box/sql/wherecode.c | 234 ------------------------------------------------ > 12 files changed, 11 insertions(+), 435 deletions(-) > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints 2018-06-09 21:41 ` [tarantool-patches] " Vladislav Shpilevoy @ 2018-06-13 6:43 ` Kirill Shcherbatov 2018-06-13 8:48 ` Vladislav Shpilevoy 0 siblings, 1 reply; 11+ messages in thread From: Kirill Shcherbatov @ 2018-06-13 6:43 UTC (permalink / raw) To: tarantool-patches; +Cc: v.shpilevoy > 1. The same comment. What have you write after '---'? Ok, I'll try to do not forget about it in next time. It were not obvious for me to edit ./create_commits.sh result files. Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3121-useless-hints Issue: https://github.com/tarantool/tarantool/issues/3121 > 2. I have pushed my fixes as a separate commit. Please, > look, perceive and squash. Done. Tnx. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints 2018-06-13 6:43 ` [tarantool-patches] " Kirill Shcherbatov @ 2018-06-13 8:48 ` Vladislav Shpilevoy 2018-06-19 9:41 ` Kirill Shcherbatov 0 siblings, 1 reply; 11+ messages in thread From: Vladislav Shpilevoy @ 2018-06-13 8:48 UTC (permalink / raw) To: tarantool-patches, Kirill Shcherbatov Hello. Thanks for the fixes! LGTM. On 13/06/2018 09:43, Kirill Shcherbatov wrote: >> 1. The same comment. What have you write after '---'? > Ok, I'll try to do not forget about it in next time. It were not obvious for me to edit ./create_commits.sh result files. ./create_commits.sh is not a standard util. It is just my helper for multi-commit patches. If you know how, you can implement Branch/Issue adding for single-commit patches, and I will share it. > > Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3121-useless-hints > Issue: https://github.com/tarantool/tarantool/issues/3121 > >> 2. I have pushed my fixes as a separate commit. Please, >> look, perceive and squash. > Done. Tnx. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints 2018-06-13 8:48 ` Vladislav Shpilevoy @ 2018-06-19 9:41 ` Kirill Shcherbatov 2018-06-19 14:03 ` n.pettik 0 siblings, 1 reply; 11+ messages in thread From: Kirill Shcherbatov @ 2018-06-19 9:41 UTC (permalink / raw) To: tarantool-patches, n.pettik Nikita, please take a look. On 13.06.2018 11:48, Vladislav Shpilevoy wrote: > Hello. Thanks for the fixes! > > LGTM. > > On 13/06/2018 09:43, Kirill Shcherbatov wrote: >>> 1. The same comment. What have you write after '---'? >> Ok, I'll try to do not forget about it in next time. It were not obvious for me to edit ./create_commits.sh result files. > > ./create_commits.sh is not a standard util. It is just my helper for multi-commit > patches. If you know how, you can implement Branch/Issue adding for single-commit > patches, and I will share it. > >> >> Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3121-useless-hints >> Issue: https://github.com/tarantool/tarantool/issues/3121 >> >>> 2. I have pushed my fixes as a separate commit. Please, >>> look, perceive and squash. >> Done. Tnx. >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints 2018-06-19 9:41 ` Kirill Shcherbatov @ 2018-06-19 14:03 ` n.pettik 2018-06-19 14:31 ` Kirill Shcherbatov 0 siblings, 1 reply; 11+ messages in thread From: n.pettik @ 2018-06-19 14:03 UTC (permalink / raw) To: tarantool-patches; +Cc: Kirill Shcherbatov > On 19 Jun 2018, at 12:41, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote: > > Nikita, please take a look. Could you please resend fresh diff after fixes, so that I review updated version. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints 2018-06-19 14:03 ` n.pettik @ 2018-06-19 14:31 ` Kirill Shcherbatov 2018-06-19 18:31 ` n.pettik 0 siblings, 1 reply; 11+ messages in thread From: Kirill Shcherbatov @ 2018-06-19 14:31 UTC (permalink / raw) To: tarantool-patches, n.pettik > Could you please resend fresh diff after fixes, so that I review > updated version. > Deleted BTREE_BULKLOAD and OPFLAG_BULKCSR as there become useless since btree.c was dropped as a part of #2419. Removed SQLITE_ENABLE_CURSOR_HINTS sections as this code has never been in use. Start use OPFLAG_SEEKEQ instead of equal BTREE_SEEK_EQ as this macro name has no sense since no btree present in SQL. Resolves #3121. --- src/box/sql.c | 4 +- src/box/sql/build.c | 4 +- src/box/sql/cursor.c | 2 +- src/box/sql/cursor.h | 16 ---- src/box/sql/expr.c | 18 ---- src/box/sql/insert.c | 1 - src/box/sql/sqliteInt.h | 7 -- src/box/sql/vdbe.c | 19 ++-- src/box/sql/vdbe.h | 4 - src/box/sql/vdbeaux.c | 133 --------------------------- src/box/sql/where.c | 10 +- src/box/sql/wherecode.c | 238 ------------------------------------------------ 12 files changed, 12 insertions(+), 444 deletions(-) diff --git a/src/box/sql.c b/src/box/sql.c index 7379cb4..c3096b2 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -292,12 +292,12 @@ int tarantoolSqlite3MovetoUnpacked(BtCursor *pCur, UnpackedRecord *pIdxKey, res_success = -1; /* item<key */ break; case OP_SeekLE: - pCur->iter_type = (pCur->hints & BTREE_SEEK_EQ) ? + pCur->iter_type = (pCur->hints & OPFLAG_SEEKEQ) != 0 ? ITER_REQ : ITER_LE; res_success = 0; /* item==key */ break; case OP_SeekGE: - pCur->iter_type = (pCur->hints & BTREE_SEEK_EQ) ? + pCur->iter_type = (pCur->hints & OPFLAG_SEEKEQ) != 0 ? ITER_EQ : ITER_GE; res_success = 0; /* item==key */ break; diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 28e4d7a..93bf0cc 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2676,9 +2676,7 @@ sqlite3RefillIndex(Parse * pParse, Index * pIndex, int memRootPage) sqlite3VdbeAddOp2(v, OP_Clear, SQLITE_PAGENO_TO_SPACEID(tnum), 0); emit_open_cursor(pParse, iIdx, tnum); - sqlite3VdbeChangeP5(v, - OPFLAG_BULKCSR | ((memRootPage >= 0) ? - OPFLAG_P2ISREG : 0)); + sqlite3VdbeChangeP5(v, memRootPage >= 0 ? OPFLAG_P2ISREG : 0); addr1 = sqlite3VdbeAddOp2(v, OP_SorterSort, iSorter, 0); VdbeCoverage(v); diff --git a/src/box/sql/cursor.c b/src/box/sql/cursor.c index 82ad08b..9dd2b02 100644 --- a/src/box/sql/cursor.c +++ b/src/box/sql/cursor.c @@ -53,7 +53,7 @@ sql_cursor_cleanup(struct BtCursor *cursor) void sqlite3CursorHintFlags(BtCursor * pCur, unsigned x) { - assert(x == BTREE_SEEK_EQ || x == BTREE_BULKLOAD || x == 0); + assert(x == OPFLAG_SEEKEQ || x == 0); pCur->hints = x; } diff --git a/src/box/sql/cursor.h b/src/box/sql/cursor.h index c559417..252aaac 100644 --- a/src/box/sql/cursor.h +++ b/src/box/sql/cursor.h @@ -35,22 +35,6 @@ typedef struct BtCursor BtCursor; /* - * Values that may be OR'd together to form the argument to the - * BTREE_HINT_FLAGS hint for sqlite3BtreeCursorHint(): - * - * The BTREE_BULKLOAD flag is set on index cursors when the index is going - * to be filled with content that is already in sorted order. - * - * The BTREE_SEEK_EQ flag is set on cursors that will get OP_SeekGE or - * OP_SeekLE opcodes for a range search, but where the range of entries - * selected will all have the same key. In other words, the cursor will - * be used only for equality key searches. - * - */ -#define BTREE_BULKLOAD 0x00000001 /* Used to full index in sorted order */ -#define BTREE_SEEK_EQ 0x00000002 /* EQ seeks only - no range seeks */ - -/* * A cursor contains a particular entry either from Tarantrool or * Sorter. Tarantool cursor is able to point to ordinary table or * ephemeral one. To distinguish them curFlags is set to TaCursor diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 8866f6f..371d5d5 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -2000,24 +2000,6 @@ sqlite3ExprIsConstantOrFunction(Expr * p, u8 isInit) return exprIsConst(p, 4 + isInit, 0); } -#ifdef SQLITE_ENABLE_CURSOR_HINTS -/* - * Walk an expression tree. Return 1 if the expression contains a - * subquery of some kind. Return 0 if there are no subqueries. - */ -int -sqlite3ExprContainsSubquery(Expr * p) -{ - Walker w; - memset(&w, 0, sizeof(w)); - w.eCode = 1; - w.xExprCallback = sqlite3ExprWalkNoop; - w.xSelectCallback = selectNodeIsConstant; - sqlite3WalkExpr(&w, p); - return w.eCode == 0; -} -#endif - /* * If the expression p codes a constant integer that is small enough * to fit in a 32-bit integer, return 1 and put the value of the integer diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 59c61c7..a3a5b45 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -1963,7 +1963,6 @@ xferOptimization(Parse * pParse, /* Parser context */ VdbeComment((v, "%s", pSrcIdx->zName)); emit_open_cursor(pParse, iDest, pDestIdx->tnum); sql_vdbe_set_p4_key_def(pParse, pDestIdx); - sqlite3VdbeChangeP5(v, OPFLAG_BULKCSR); VdbeComment((v, "%s", pDestIdx->zName)); addr1 = sqlite3VdbeAddOp2(v, OP_Rewind, iSrc, 0); VdbeCoverage(v); diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 01351a1..272d95e 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -2978,8 +2978,6 @@ struct Parse { * Value constraints (enforced via assert()): * OPFLAG_LENGTHARG == SQLITE_FUNC_LENGTH * OPFLAG_TYPEOFARG == SQLITE_FUNC_TYPEOF - * OPFLAG_BULKCSR == BTREE_BULKLOAD - * OPFLAG_SEEKEQ == BTREE_SEEK_EQ * OPFLAG_FORDELETE == BTREE_FORDELETE * OPFLAG_SAVEPOSITION == BTREE_SAVEPOSITION * OPFLAG_AUXDELETE == BTREE_AUXDELETE @@ -2995,7 +2993,6 @@ struct Parse { #endif #define OPFLAG_LENGTHARG 0x40 /* OP_Column only used for length() */ #define OPFLAG_TYPEOFARG 0x80 /* OP_Column only used for typeof() */ -#define OPFLAG_BULKCSR 0x01 /* OP_Open** used to open bulk cursor */ #define OPFLAG_SEEKEQ 0x02 /* OP_Open** cursor uses EQ seek only */ #define OPFLAG_FORDELETE 0x08 /* OP_Open should use BTREE_FORDELETE */ #define OPFLAG_P2ISREG 0x10 /* P2 to OP_Open** is a register number */ @@ -3239,7 +3236,6 @@ struct Walker { int iCur; /* A cursor number */ SrcList *pSrcList; /* FROM clause */ struct SrcCount *pSrcCount; /* Counting column references */ - struct CCurHint *pCCurHint; /* Used by codeCursorHint() */ int *aiCol; /* array of column indexes */ struct IdxCover *pIdxCover; /* Check for index coverage */ /** Space definition. */ @@ -3834,9 +3830,6 @@ int sqlite3ExprIsConstant(Expr *); int sqlite3ExprIsConstantNotJoin(Expr *); int sqlite3ExprIsConstantOrFunction(Expr *, u8); int sqlite3ExprIsTableConstant(Expr *, int); -#ifdef SQLITE_ENABLE_CURSOR_HINTS -int sqlite3ExprContainsSubquery(Expr *); -#endif int sqlite3ExprIsInteger(Expr *, int *); int sqlite3ExprCanBeNull(const Expr *); int sqlite3ExprNeedsNoAffinityChange(const Expr *, char); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 3fe5875..3139933 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3206,14 +3206,7 @@ case OP_OpenWrite: pCur->key_def = index->def->key_def; open_cursor_set_hints: - assert(OPFLAG_BULKCSR==BTREE_BULKLOAD); - assert(OPFLAG_SEEKEQ==BTREE_SEEK_EQ); - testcase( pOp->p5 & OPFLAG_BULKCSR); -#ifdef SQLITE_ENABLE_CURSOR_HINTS - testcase( pOp->p2 & OPFLAG_SEEKEQ); -#endif - sqlite3CursorHintFlags(pCur->uc.pCursor, - (pOp->p5 & (OPFLAG_BULKCSR|OPFLAG_SEEKEQ))); + sqlite3CursorHintFlags(pCur->uc.pCursor, pOp->p5 & OPFLAG_SEEKEQ); if (rc) goto abort_due_to_error; break; } @@ -3527,11 +3520,13 @@ case OP_SeekGT: { /* jump, in3 */ } } } - /* For a cursor with the BTREE_SEEK_EQ hint, only the OP_SeekGE and - * OP_SeekLE opcodes are allowed, and these must be immediately followed - * by an OP_IdxGT or OP_IdxLT opcode, respectively, with the same key. + /* + * For a cursor with the OPFLAG_SEEKEQ hint, only the + * OP_SeekGE and OP_SeekLE opcodes are allowed, and these + * must be immediately followed by an OP_IdxGT or + * OP_IdxLT opcode, respectively, with the same key. */ - if (sqlite3CursorHasHint(pC->uc.pCursor, BTREE_SEEK_EQ)) { + if (sqlite3CursorHasHint(pC->uc.pCursor, OPFLAG_SEEKEQ) != 0) { eqOnly = 1; assert(pOp->opcode==OP_SeekGE || pOp->opcode==OP_SeekLE); assert(pOp[1].opcode==OP_IdxLT || pOp[1].opcode==OP_IdxGT); diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h index 68d542c..30838b6 100644 --- a/src/box/sql/vdbe.h +++ b/src/box/sql/vdbe.h @@ -80,9 +80,6 @@ struct VdbeOp { int *ai; /* Used when p4type is P4_INTARRAY */ SubProgram *pProgram; /* Used when p4type is P4_SUBPROGRAM */ Index *pIndex; /* Used when p4type is P4_INDEX */ -#ifdef SQLITE_ENABLE_CURSOR_HINTS - Expr *pExpr; /* Used when p4type is P4_EXPR */ -#endif int (*xAdvance) (BtCursor *, int *); /** Used when p4type is P4_KEYDEF. */ struct key_def *key_def; @@ -134,7 +131,6 @@ typedef struct VdbeOpList VdbeOpList; #define P4_STATIC (-2) /* Pointer to a static string */ #define P4_COLLSEQ (-3) /* P4 is a pointer to a CollSeq structure */ #define P4_FUNCDEF (-4) /* P4 is a pointer to a FuncDef structure */ -#define P4_EXPR (-6) /* P4 is a pointer to an Expr tree */ #define P4_MEM (-7) /* P4 is a pointer to a Mem* structure */ #define P4_TRANSIENT 0 /* P4 is a pointer to a transient string */ #define P4_REAL (-9) /* P4 is a 64-bit floating point value */ diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 679bd0b..a29d0a3 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -972,12 +972,6 @@ freeP4(sqlite3 * db, int p4type, void *p4) case P4_KEYDEF: key_def_delete(p4); break; -#ifdef SQLITE_ENABLE_CURSOR_HINTS - case P4_EXPR:{ - sqlite3ExprDelete(db, (Expr *) p4); - break; - } -#endif case P4_FUNCDEF:{ freeEphemeralFunction(db, (FuncDef *) p4); break; @@ -1389,127 +1383,6 @@ displayComment(const Op * pOp, /* The opcode to be commented */ } #endif /* SQLITE_DEBUG */ -#if defined(SQLITE_ENABLE_CURSOR_HINTS) -/* - * Translate the P4.pExpr value for an OP_CursorHint opcode into text - * that can be displayed in the P4 column of EXPLAIN output. - */ -static void -displayP4Expr(StrAccum * p, Expr * pExpr) -{ - const char *zOp = 0; - switch (pExpr->op) { - case TK_STRING: - sqlite3XPrintf(p, "%Q", pExpr->u.zToken); - break; - case TK_INTEGER: - sqlite3XPrintf(p, "%d", pExpr->u.iValue); - break; - case TK_NULL: - sqlite3XPrintf(p, "NULL"); - break; - case TK_REGISTER:{ - sqlite3XPrintf(p, "r[%d]", pExpr->iTable); - break; - } - case TK_COLUMN:{ - if (pExpr->iColumn < 0) { - sqlite3XPrintf(p, "rowid"); - } else { - sqlite3XPrintf(p, "c%d", (int)pExpr->iColumn); - } - break; - } - case TK_LT: - zOp = "LT"; - break; - case TK_LE: - zOp = "LE"; - break; - case TK_GT: - zOp = "GT"; - break; - case TK_GE: - zOp = "GE"; - break; - case TK_NE: - zOp = "NE"; - break; - case TK_EQ: - zOp = "EQ"; - break; - case TK_AND: - zOp = "AND"; - break; - case TK_OR: - zOp = "OR"; - break; - case TK_PLUS: - zOp = "ADD"; - break; - case TK_STAR: - zOp = "MUL"; - break; - case TK_MINUS: - zOp = "SUB"; - break; - case TK_REM: - zOp = "REM"; - break; - case TK_BITAND: - zOp = "BITAND"; - break; - case TK_BITOR: - zOp = "BITOR"; - break; - case TK_SLASH: - zOp = "DIV"; - break; - case TK_LSHIFT: - zOp = "LSHIFT"; - break; - case TK_RSHIFT: - zOp = "RSHIFT"; - break; - case TK_CONCAT: - zOp = "CONCAT"; - break; - case TK_UMINUS: - zOp = "MINUS"; - break; - case TK_UPLUS: - zOp = "PLUS"; - break; - case TK_BITNOT: - zOp = "BITNOT"; - break; - case TK_NOT: - zOp = "NOT"; - break; - case TK_ISNULL: - zOp = "IS NULL"; - break; - case TK_NOTNULL: - zOp = "NOT NULL"; - break; - - default: - sqlite3XPrintf(p, "%s", "expr"); - break; - } - - if (zOp) { - sqlite3XPrintf(p, "%s(", zOp); - displayP4Expr(p, pExpr->pLeft); - if (pExpr->pRight) { - sqlite3StrAccumAppend(p, ",", 1); - displayP4Expr(p, pExpr->pRight); - } - sqlite3StrAccumAppend(p, ")", 1); - } -} -#endif /* defined(SQLITE_ENABLE_CURSOR_HINTS) */ - /* * Compute a string that describes the P4 parameter for an opcode. * Use zTemp for any required temporary buffer space. @@ -1549,12 +1422,6 @@ displayP4(Op * pOp, char *zTemp, int nTemp) } break; } -#ifdef SQLITE_ENABLE_CURSOR_HINTS - case P4_EXPR:{ - displayP4Expr(&x, pOp->p4.pExpr); - break; - } -#endif case P4_COLLSEQ:{ struct coll *pColl = pOp->p4.pColl; if (pColl != NULL) diff --git a/src/box/sql/where.c b/src/box/sql/where.c index e791647..4026824 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -4588,15 +4588,7 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */ && pTab->nCol == BMS - 1); testcase(pWInfo->eOnePass == ONEPASS_OFF && pTab->nCol == BMS); -#ifdef SQLITE_ENABLE_CURSOR_HINTS - if (pLoop->pIndex != 0) { - sqlite3VdbeChangeP5(v, - OPFLAG_SEEKEQ | bFordelete); - } else -#endif - { - sqlite3VdbeChangeP5(v, bFordelete); - } + sqlite3VdbeChangeP5(v, bFordelete); #ifdef SQLITE_ENABLE_COLUMN_USED_MASK sqlite3VdbeAddOp4Dup8(v, OP_ColumnsUsed, pTabItem->iCursor, 0, 0, diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c index 09b2671..bbf56b3 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -817,242 +817,6 @@ whereLikeOptimizationStringFixup(Vdbe * v, /* prepared statement under construc #define whereLikeOptimizationStringFixup(A,B,C) #endif -#ifdef SQLITE_ENABLE_CURSOR_HINTS -/* - * Information is passed from codeCursorHint() down to individual nodes of - * the expression tree (by sqlite3WalkExpr()) using an instance of this - * structure. - */ -struct CCurHint { - int iTabCur; /* Cursor for the main table */ - int iIdxCur; /* Cursor for the index, if pIdx!=0. Unused otherwise */ - Index *pIdx; /* The index used to access the table */ -}; - -/* - * This function is called for every node of an expression that is a candidate - * for a cursor hint on an index cursor. For TK_COLUMN nodes that reference - * the table CCurHint.iTabCur, verify that the same column can be - * accessed through the index. If it cannot, then set pWalker->eCode to 1. - */ -static int -codeCursorHintCheckExpr(Walker * pWalker, Expr * pExpr) -{ - struct CCurHint *pHint = pWalker->u.pCCurHint; - assert(pHint->pIdx != 0); - if (pExpr->op == TK_COLUMN - && pExpr->iTable == pHint->iTabCur - && (pExpr->iColumn < 0)) { - pWalker->eCode = 1; - } - return WRC_Continue; -} - -/* - * Test whether or not expression pExpr, which was part of a WHERE clause, - * should be included in the cursor-hint for a table that is on the rhs - * of a LEFT JOIN. Set Walker.eCode to non-zero before returning if the - * expression is not suitable. - * - * An expression is unsuitable if it might evaluate to non NULL even if - * a TK_COLUMN node that does affect the value of the expression is set - * to NULL. For example: - * - * col IS NULL - * col IS NOT NULL - * coalesce(col, 1) - * CASE WHEN col THEN 0 ELSE 1 END - */ -static int -codeCursorHintIsOrFunction(Walker * pWalker, Expr * pExpr) -{ - if (pExpr->op == TK_ISNULL - || pExpr->op == TK_NOTNULL || pExpr->op == TK_CASE) { - pWalker->eCode = 1; - } else if (pExpr->op == TK_FUNCTION) { - int d1; - char d2[3]; - if (0 == - sqlite3IsLikeFunction(pWalker->pParse->db, pExpr, &d1, - d2)) { - pWalker->eCode = 1; - } - } - - return WRC_Continue; -} - -/* - * This function is called on every node of an expression tree used as an - * argument to the OP_CursorHint instruction. If the node is a TK_COLUMN - * that accesses any table other than the one identified by - * CCurHint.iTabCur, then do the following: - * - * 1) allocate a register and code an OP_Column instruction to read - * the specified column into the new register, and - * - * 2) transform the expression node to a TK_REGISTER node that reads - * from the newly populated register. - * - * Also, if the node is a TK_COLUMN that does access the table idenified - * by pCCurHint.iTabCur, and an index is being used (which we will - * know because CCurHint.pIdx!=0) then transform the TK_COLUMN into - * an access of the index rather than the original table. - */ -static int -codeCursorHintFixExpr(Walker * pWalker, Expr * pExpr) -{ - int rc = WRC_Continue; - struct CCurHint *pHint = pWalker->u.pCCurHint; - if (pExpr->op == TK_COLUMN) { - if (pExpr->iTable != pHint->iTabCur) { - Vdbe *v = pWalker->pParse->pVdbe; - int reg = ++pWalker->pParse->nMem; /* Register for column value */ - sqlite3ExprCodeGetColumnOfTable(v, pExpr->pTab->def, - pExpr->iTable, - pExpr->iColumn, reg); - pExpr->op = TK_REGISTER; - pExpr->iTable = reg; - } else if (pHint->pIdx != 0) { - pExpr->iTable = pHint->iIdxCur; - assert(pExpr->iColumn >= 0); - } - } else if (pExpr->op == TK_AGG_FUNCTION) { - /* An aggregate function in the WHERE clause of a query means this must - * be a correlated sub-query, and expression pExpr is an aggregate from - * the parent context. Do not walk the function arguments in this case. - * - * todo: It should be possible to replace this node with a TK_REGISTER - * expression, as the result of the expression must be stored in a - * register at this point. The same holds for TK_AGG_COLUMN nodes. - */ - rc = WRC_Prune; - } - return rc; -} - -/* - * Insert an OP_CursorHint instruction if it is appropriate to do so. - */ -static void -codeCursorHint(struct SrcList_item *pTabItem, /* FROM clause item */ - WhereInfo * pWInfo, /* The where clause */ - WhereLevel * pLevel, /* Which loop to provide hints for */ - WhereTerm * pEndRange) /* Hint this end-of-scan boundary term if not NULL */ -{ - Parse *pParse = pWInfo->pParse; - sqlite3 *db = pParse->db; - Vdbe *v = pParse->pVdbe; - Expr *pExpr = 0; - WhereLoop *pLoop = pLevel->pWLoop; - int iCur; - WhereClause *pWC; - WhereTerm *pTerm; - int i, j; - struct CCurHint sHint; - Walker sWalker; - - if (OptimizationDisabled(db, SQLITE_CursorHints)) - return; - iCur = pLevel->iTabCur; - assert(iCur == pWInfo->pTabList->a[pLevel->iFrom].iCursor); - sHint.iTabCur = iCur; - sHint.iIdxCur = pLevel->iIdxCur; - sHint.pIdx = pLoop->pIndex; - memset(&sWalker, 0, sizeof(sWalker)); - sWalker.pParse = pParse; - sWalker.u.pCCurHint = &sHint; - pWC = &pWInfo->sWC; - for (i = 0; i < pWC->nTerm; i++) { - pTerm = &pWC->a[i]; - if (pTerm->wtFlags & (TERM_VIRTUAL | TERM_CODED)) - continue; - if (pTerm->prereqAll & pLevel->notReady) - continue; - - /* Any terms specified as part of the ON(...) clause for any LEFT - * JOIN for which the current table is not the rhs are omitted - * from the cursor-hint. - * - * If this table is the rhs of a LEFT JOIN, "IS" or "IS NULL" terms - * that were specified as part of the WHERE clause must be excluded. - * This is to address the following: - * - * SELECT ... t1 LEFT JOIN t2 ON (t1.a=t2.b) WHERE t2.c IS NULL; - * - * Say there is a single row in t2 that matches (t1.a=t2.b), but its - * t2.c values is not NULL. If the (t2.c IS NULL) constraint is - * pushed down to the cursor, this row is filtered out, causing - * SQLite to synthesize a row of NULL values. Which does match the - * WHERE clause, and so the query returns a row. Which is incorrect. - * - * For the same reason, WHERE terms such as: - * - * WHERE 1 = (t2.c IS NULL) - * - * are also excluded. See codeCursorHintIsOrFunction() for details. - */ - if (pTabItem->fg.jointype & JT_LEFT) { - Expr *pExpr = pTerm->pExpr; - if (!ExprHasProperty(pExpr, EP_FromJoin) - || pExpr->iRightJoinTable != pTabItem->iCursor) { - sWalker.eCode = 0; - sWalker.xExprCallback = - codeCursorHintIsOrFunction; - sqlite3WalkExpr(&sWalker, pTerm->pExpr); - if (sWalker.eCode) - continue; - } - } else { - if (ExprHasProperty(pTerm->pExpr, EP_FromJoin)) - continue; - } - - /* All terms in pWLoop->aLTerm[] except pEndRange are used to initialize - * the cursor. These terms are not needed as hints for a pure range - * scan (that has no == terms) so omit them. - */ - if (pLoop->nEq == 0 && pTerm != pEndRange) { - for (j = 0; - j < pLoop->nLTerm && pLoop->aLTerm[j] != pTerm; - j++) { - } - if (j < pLoop->nLTerm) - continue; - } - - /* No subqueries or non-deterministic functions allowed */ - if (sqlite3ExprContainsSubquery(pTerm->pExpr)) - continue; - - /* For an index scan, make sure referenced columns are actually in - * the index. - */ - if (sHint.pIdx != 0) { - sWalker.eCode = 0; - sWalker.xExprCallback = codeCursorHintCheckExpr; - sqlite3WalkExpr(&sWalker, pTerm->pExpr); - if (sWalker.eCode) - continue; - } - - /* If we survive all prior tests, that means this term is worth hinting */ - pExpr = - sqlite3ExprAnd(db, pExpr, - sqlite3ExprDup(db, pTerm->pExpr, 0)); - } - if (pExpr != 0) { - sWalker.xExprCallback = codeCursorHintFixExpr; - sqlite3WalkExpr(&sWalker, pExpr); - sqlite3VdbeAddOp4(v, OP_CursorHint, - (sHint.pIdx ? sHint.iIdxCur : sHint.iTabCur), - 0, 0, (const char *)pExpr, P4_EXPR); - } -} -#else -#define codeCursorHint(A,B,C,D) /* No-op */ -#endif /* SQLITE_ENABLE_CURSOR_HINTS */ - /* * If the expression passed as the second argument is a vector, generate * code to write the first nReg elements of the vector into an array @@ -1332,7 +1096,6 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about t * and store the values of those terms in an array of registers * starting at regBase. */ - codeCursorHint(pTabItem, pWInfo, pLevel, pRangeEnd); regBase = codeAllEqualityTerms(pParse, pLevel, bRev, nExtraReg, &zStartAff); @@ -1842,7 +1605,6 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about t */ pLevel->op = OP_Noop; } else { - codeCursorHint(pTabItem, pWInfo, pLevel, 0); pLevel->op = aStep[bRev]; pLevel->p1 = iCur; pLevel->p2 = -- 2.7.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints 2018-06-19 14:31 ` Kirill Shcherbatov @ 2018-06-19 18:31 ` n.pettik 2018-06-20 6:58 ` Kirill Shcherbatov 0 siblings, 1 reply; 11+ messages in thread From: n.pettik @ 2018-06-19 18:31 UTC (permalink / raw) To: tarantool-patches; +Cc: Kirill Shcherbatov > On 19 Jun 2018, at 17:31, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote: > >> Could you please resend fresh diff after fixes, so that I review >> updated version. >> > > Deleted BTREE_BULKLOAD and OPFLAG_BULKCSR as there become Typo: there —> they. > useless since btree.c was dropped as a part of #2419. > Removed SQLITE_ENABLE_CURSOR_HINTS sections as this code > has never been in use. > Start use OPFLAG_SEEKEQ instead of equal BTREE_SEEK_EQ as > this macro name has no sense since no btree present in SQL. > > Resolves #3121. Except for your removals, I also suggest you to remove: SQLITE_CursorHints macros; sqlite3CursorHintFlags() function — it is used only once and can be inlined; sqlite3CursorHasHint() function — it is used only once and can be inlined; ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints 2018-06-19 18:31 ` n.pettik @ 2018-06-20 6:58 ` Kirill Shcherbatov 2018-06-20 9:05 ` n.pettik 0 siblings, 1 reply; 11+ messages in thread From: Kirill Shcherbatov @ 2018-06-20 6:58 UTC (permalink / raw) To: tarantool-patches; +Cc: Nikita Pettik >> Deleted BTREE_BULKLOAD and OPFLAG_BULKCSR as there become > > Typo: there —> they. Ok, fixed. > Except for your removals, I also suggest you to remove: > > SQLITE_CursorHints macros; > sqlite3CursorHintFlags() function — it is used only once and can be inlined; > sqlite3CursorHasHint() function — it is used only once and can be inlined; Thank you for suggestion, diff --git a/src/box/sql/cursor.c b/src/box/sql/cursor.c index 9dd2b02..b096691 100644 --- a/src/box/sql/cursor.c +++ b/src/box/sql/cursor.c @@ -48,16 +48,6 @@ sql_cursor_cleanup(struct BtCursor *cursor) } /* - * Provide flag hints to the cursor. - */ -void -sqlite3CursorHintFlags(BtCursor * pCur, unsigned x) -{ - assert(x == OPFLAG_SEEKEQ || x == 0); - pCur->hints = x; -} - -/* * Initialize memory that will be converted into a BtCursor object. */ void @@ -185,13 +175,3 @@ sqlite3CursorPrevious(BtCursor *pCur, int *pRes) *pRes = 0; return tarantoolSqlite3Previous(pCur, pRes); } - -/* - * Return true if the cursor has a hint specified. This routine is - * only used from within assert() statements - */ -int -sqlite3CursorHasHint(BtCursor *pCsr, unsigned int mask) -{ - return (pCsr->hints & mask) != 0; -} diff --git a/src/box/sql/cursor.h b/src/box/sql/cursor.h index 252aaac..da711a7 100644 --- a/src/box/sql/cursor.h +++ b/src/box/sql/cursor.h @@ -54,7 +54,6 @@ struct BtCursor { }; void sqlite3CursorZero(BtCursor *); -void sqlite3CursorHintFlags(BtCursor *, unsigned); /** * Close a cursor and invalidate its state. In case of @@ -76,7 +75,6 @@ int sqlite3CursorPayload(BtCursor *, u32 offset, u32 amt, void *); */ void sql_cursor_cleanup(struct BtCursor *cursor); -int sqlite3CursorHasHint(BtCursor *, unsigned int mask); #ifndef NDEBUG int sqlite3CursorIsValid(BtCursor *); diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 272d95e..956d6fc 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1696,7 +1696,6 @@ struct sqlite3 { #define SQLITE_SubqCoroutine 0x0100 /* Evaluate subqueries as coroutines */ #define SQLITE_Transitive 0x0200 /* Transitive constraints */ #define SQLITE_OmitNoopJoin 0x0400 /* Omit unused tables in joins */ -#define SQLITE_CursorHints 0x2000 /* Add OP_CursorHint opcodes */ #define SQLITE_AllOpts 0xffff /* All optimizations */ /* diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 3139933..e1265f4 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3206,7 +3206,7 @@ case OP_OpenWrite: pCur->key_def = index->def->key_def; open_cursor_set_hints: - sqlite3CursorHintFlags(pCur->uc.pCursor, pOp->p5 & OPFLAG_SEEKEQ); + pCur->uc.pCursor->hints = pOp->p5 & OPFLAG_SEEKEQ; if (rc) goto abort_due_to_error; break; } @@ -3526,7 +3526,7 @@ case OP_SeekGT: { /* jump, in3 */ * must be immediately followed by an OP_IdxGT or * OP_IdxLT opcode, respectively, with the same key. */ - if (sqlite3CursorHasHint(pC->uc.pCursor, OPFLAG_SEEKEQ) != 0) { + if ((pC->uc.pCursor->hints & OPFLAG_SEEKEQ) != 0) { eqOnly = 1; assert(pOp->opcode==OP_SeekGE || pOp->opcode==OP_SeekLE); assert(pOp[1].opcode==OP_IdxLT || pOp[1].opcode==OP_IdxGT); ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints 2018-06-20 6:58 ` Kirill Shcherbatov @ 2018-06-20 9:05 ` n.pettik 2018-06-22 12:11 ` Kirill Yukhin 0 siblings, 1 reply; 11+ messages in thread From: n.pettik @ 2018-06-20 9:05 UTC (permalink / raw) To: tarantool-patches; +Cc: Kirill Shcherbatov LGTM. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints 2018-06-20 9:05 ` n.pettik @ 2018-06-22 12:11 ` Kirill Yukhin 0 siblings, 0 replies; 11+ messages in thread From: Kirill Yukhin @ 2018-06-22 12:11 UTC (permalink / raw) To: tarantool-patches; +Cc: Kirill Shcherbatov Hello, On 20 июн 12:05, n.pettik wrote: > LGTM. I've checked in the patch into 2.0 branch. NB: couple of sql-tap tests fails due to not-correlated reason. Patch to fix this is under review. -- Regards, Kirill Yukhin ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-06-22 12:16 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-09 13:09 [tarantool-patches] [PATCH v1 1/1] sql: drop useless legacy Cursor hints Kirill Shcherbatov 2018-06-09 21:41 ` [tarantool-patches] " Vladislav Shpilevoy 2018-06-13 6:43 ` [tarantool-patches] " Kirill Shcherbatov 2018-06-13 8:48 ` Vladislav Shpilevoy 2018-06-19 9:41 ` Kirill Shcherbatov 2018-06-19 14:03 ` n.pettik 2018-06-19 14:31 ` Kirill Shcherbatov 2018-06-19 18:31 ` n.pettik 2018-06-20 6:58 ` Kirill Shcherbatov 2018-06-20 9:05 ` n.pettik 2018-06-22 12:11 ` Kirill Yukhin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox