[tarantool-patches] [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[tarantool-patches] [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[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

[tarantool-patches] Re: [tarantool-patches] [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[Vladislav Shpilevoy]

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(-)
>

[tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[Kirill Shcherbatov]

> 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.

[tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[Vladislav Shpilevoy]

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.
> 

[tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[Kirill Shcherbatov]

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.
>>
> 

[tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[n.pettik]

> 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.

[tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[Kirill Shcherbatov]

> 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

[tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[n.pettik]

> 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;

 

[tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[Kirill Shcherbatov]

>> 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);

[tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[n.pettik]

LGTM.

[tarantool-patches] Re: [PATCH v1 1/1] sql: drop useless legacy Cursor hints

[Kirill Yukhin]

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