Tarantool development patches archive
 help / color / mirror / Atom feed
From: Kirill Yukhin <kyukhin@tarantool.org>
To: v.shpilevoy@tarantool.org
Cc: tarantool-patches@freelists.org, Kirill Yukhin <kyukhin@tarantool.org>
Subject: [tarantool-patches] [PATCH 2/3] sql: remove expressions from SQL indexes
Date: Fri,  1 Jun 2018 18:16:11 +0300	[thread overview]
Message-ID: <787e55ddaaeeec54e870d3f493fc235fe64ffd51.1527865931.git.kyukhin@tarantool.org> (raw)
In-Reply-To: <cover.1527865931.git.kyukhin@tarantool.org>
In-Reply-To: <cover.1527865931.git.kyukhin@tarantool.org>

Legacy SQL FE was able to create indexes w/ expressions.
Tarantool will employ diofferenct scheme to implement
functional indexes, thus code  handling it in SQL FE is
dead and redundant. Remove it.

Part of #3235
---
 src/box/sql/build.c     | 22 ++++++---------
 src/box/sql/delete.c    |  3 +--
 src/box/sql/expr.c      | 12 ++-------
 src/box/sql/insert.c    | 72 +++++++++++++++----------------------------------
 src/box/sql/sqliteInt.h |  6 -----
 src/box/sql/where.c     | 27 ++-----------------
 src/box/sql/wherecode.c | 13 +++------
 src/box/sql/whereexpr.c | 36 +++----------------------
 8 files changed, 42 insertions(+), 149 deletions(-)

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 28e4d7a..65bba1f 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -252,7 +252,6 @@ static void
 freeIndex(sqlite3 * db, Index * p)
 {
 	sql_expr_delete(db, p->pPartIdxWhere, false);
-	sql_expr_list_delete(db, p->aColExpr);
 	sqlite3DbFree(db, p->zColAff);
 	sqlite3DbFree(db, p);
 }
@@ -3042,8 +3041,7 @@ sql_create_index(struct Parse *parse, struct Token *token,
 
 	/* Analyze the list of expressions that form the terms of the index and
 	 * report any errors.  In the common case where the expression is exactly
-	 * a table column, store that column in aiColumn[].  For general expressions,
-	 * populate pIndex->aColExpr and store XN_EXPR (-2) in aiColumn[].
+	 * a table column, store that column in aiColumn[].
 	 *
 	 * TODO: Issue a warning if two or more columns of the index are identical.
 	 * TODO: Issue a warning if the table primary key is used as part of the
@@ -3938,17 +3936,13 @@ sqlite3UniqueConstraint(Parse * pParse,	/* Parsing context */
 	Table *pTab = pIdx->pTable;
 
 	sqlite3StrAccumInit(&errMsg, pParse->db, 0, 0, 200);
-	if (pIdx->aColExpr) {
-		sqlite3XPrintf(&errMsg, "index '%q'", pIdx->zName);
-	} else {
-		for (j = 0; j < pIdx->nColumn; j++) {
-			char *zCol;
-			assert(pIdx->aiColumn[j] >= 0);
-			zCol = pTab->def->fields[pIdx->aiColumn[j]].name;
-			if (j)
-				sqlite3StrAccumAppend(&errMsg, ", ", 2);
-			sqlite3XPrintf(&errMsg, "%s.%s", pTab->def->name, zCol);
-		}
+	for (j = 0; j < pIdx->nColumn; j++) {
+		char *zCol;
+		assert(pIdx->aiColumn[j] >= 0);
+		zCol = pTab->def->fields[pIdx->aiColumn[j]].name;
+		if (j)
+			sqlite3StrAccumAppend(&errMsg, ", ", 2);
+		sqlite3XPrintf(&errMsg, "%s.%s", pTab->def->name, zCol);
 	}
 	zErr = sqlite3StrAccumFinish(&errMsg);
 	sqlite3HaltConstraint(pParse,
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 28713c8..2b59130 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -564,8 +564,7 @@ sql_generate_index_key(struct Parse *parse, struct Index *index, int cursor,
 			     prev->pPartIdxWhere != NULL))
 		prev = NULL;
 	for (int j = 0; j < col_cnt; j++) {
-		if (prev != NULL && prev->aiColumn[j] == index->aiColumn[j]
-		    && prev->aiColumn[j] != XN_EXPR) {
+		if (prev != NULL && prev->aiColumn[j] == index->aiColumn[j]) {
 			/*
 			 * This column was already computed by the
 			 * previous index.
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 8866f6f..a69d38b 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -3484,16 +3484,8 @@ sqlite3ExprCodeLoadIndexColumn(Parse * pParse,	/* The parsing context */
     )
 {
 	i16 iTabCol = pIdx->aiColumn[iIdxCol];
-	if (iTabCol == XN_EXPR) {
-		assert(pIdx->aColExpr);
-		assert(pIdx->aColExpr->nExpr > iIdxCol);
-		pParse->iSelfTab = iTabCur;
-		sqlite3ExprCodeCopy(pParse, pIdx->aColExpr->a[iIdxCol].pExpr,
-				    regOut);
-	} else {
-		sqlite3ExprCodeGetColumnOfTable(pParse->pVdbe, pIdx->pTable->def,
-						iTabCur, iTabCol, regOut);
-	}
+	sqlite3ExprCodeGetColumnOfTable(pParse->pVdbe, pIdx->pTable->def,
+					iTabCur, iTabCol, regOut);
 }
 
 void
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 59c61c7..3dbf855 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -98,21 +98,9 @@ sqlite3IndexAffinityStr(sqlite3 * db, Index * pIdx)
 		}
 		for (n = 0; n < nColumn; n++) {
 			i16 x = pIdx->aiColumn[n];
-			if (x >= 0) {
-				char affinity = pIdx->pTable->
-					def->fields[x].affinity;
-				pIdx->zColAff[n] = affinity;
-			} else {
-				char aff;
-				assert(x == XN_EXPR);
-				assert(pIdx->aColExpr != 0);
-				aff =
-				    sqlite3ExprAffinity(pIdx->aColExpr->a[n].
-							pExpr);
-				if (aff == 0)
-					aff = AFFINITY_BLOB;
-				pIdx->zColAff[n] = aff;
-			}
+			char affinity = pIdx->pTable->
+				def->fields[x].affinity;
+			pIdx->zColAff[n] = affinity;
 		}
 		pIdx->zColAff[n] = 0;
 	}
@@ -1256,34 +1244,24 @@ sqlite3GenerateConstraintChecks(Parse * pParse,		/* The parser context */
 		for (i = 0; i < nIdxCol; i++) {
 			int iField = pIdx->aiColumn[i];
 			int x;
-			if (iField == XN_EXPR) {
-				pParse->ckBase = regNewData + 1;
-				sqlite3ExprCodeCopy(pParse,
-						    pIdx->aColExpr->a[i].pExpr,
-						    regIdx + i);
-				pParse->ckBase = 0;
-				VdbeComment((v, "%s column %d", pIdx->zName,
-					     i));
-			} else {
-				/* OP_SCopy copies value in separate register,
-				 * which later will be used by OP_NoConflict.
-				 * But OP_NoConflict is necessary only in cases
-				 * when bytecode is needed for proper UNIQUE
-				 * constraint handling.
-				 */
-				if (uniqueByteCodeNeeded) {
-					if (iField == pTab->iPKey)
-						x = regNewData;
-					else
-						x = iField + regNewData + 1;
-
-					assert(iField >= 0);
-					sqlite3VdbeAddOp2(v, OP_SCopy,
-							  x, regIdx + i);
-					VdbeComment((v, "%s",
-						     pTab->def->fields[
-							iField].name));
-				}
+			/* OP_SCopy copies value in separate register,
+			 * which later will be used by OP_NoConflict.
+			 * But OP_NoConflict is necessary only in cases
+			 * when bytecode is needed for proper UNIQUE
+			 * constraint handling.
+			 */
+			if (uniqueByteCodeNeeded) {
+				if (iField == pTab->iPKey)
+					x = regNewData;
+				else
+					x = iField + regNewData + 1;
+
+				assert(iField >= 0);
+				sqlite3VdbeAddOp2(v, OP_SCopy,
+						  x, regIdx + i);
+				VdbeComment((v, "%s",
+					     pTab->def->fields[
+						     iField].name));
 			}
 		}
 
@@ -1688,14 +1666,6 @@ xferCompatibleIndex(Index * pDest, Index * pSrc)
 		if (pSrc->aiColumn[i] != pDest->aiColumn[i]) {
 			return 0;	/* Different columns indexed */
 		}
-		if (pSrc->aiColumn[i] == XN_EXPR) {
-			assert(pSrc->aColExpr != 0 && pDest->aColExpr != 0);
-			if (sqlite3ExprCompare(pSrc->aColExpr->a[i].pExpr,
-					       pDest->aColExpr->a[i].pExpr,
-					       -1) != 0) {
-				return 0;	/* Different expressions in the index */
-			}
-		}
 		if (sql_index_column_sort_order(pSrc, i) !=
 		    sql_index_column_sort_order(pDest, i)) {
 			return 0;	/* Different sort orders */
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 01351a1..943fda9 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2116,7 +2116,6 @@ struct Index {
 	/** Array of collation identifiers. */
 	uint32_t *coll_id_array;
 	Expr *pPartIdxWhere;	/* WHERE clause for partial indices */
-	ExprList *aColExpr;	/* Column expressions */
 	int tnum;		/* DB Page containing root of this index */
 	u16 nColumn;		/* Number of columns stored in the index */
 	u8 onError;		/* ON_CONFLICT_ACTION_ABORT, _IGNORE, _REPLACE,
@@ -2161,11 +2160,6 @@ index_field_tuple_est(struct Index *idx, uint32_t field);
 #define IsUniqueIndex(X)      (((X)->idxType == SQLITE_IDXTYPE_UNIQUE) || \
 				((X)->idxType == SQLITE_IDXTYPE_PRIMARYKEY))
 
-/* The Index.aiColumn[] values are normally positive integer.  But
- * there are some negative values that have special meaning:
- */
-#define XN_EXPR      (-2)	/* Indexed column is an expression */
-
 #ifdef DEFAULT_TUPLE_COUNT
 #undef DEFAULT_TUPLE_COUNT
 #endif
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index e791647..d312587 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -265,11 +265,6 @@ whereScanNext(WhereScan * pScan)
 			for (pTerm = pWC->a + k; k < pWC->nTerm; k++, pTerm++) {
 				if (pTerm->leftCursor == iCur
 				    && pTerm->u.leftColumn == iColumn
-				    && (iColumn != XN_EXPR
-					|| sqlite3ExprCompare(pTerm->pExpr->
-							      pLeft,
-							      pScan->pIdxExpr,
-							      iCur) == 0)
 				    && (pScan->iEquiv <= 1
 					|| !ExprHasProperty(pTerm->pExpr,
 							    EP_FromJoin))
@@ -377,9 +372,7 @@ whereScanInit(WhereScan * pScan,	/* The WhereScan object being initialized */
 	if (pIdx) {
 		int j = iColumn;
 		iColumn = pIdx->aiColumn[j];
-		if (iColumn == XN_EXPR) {
-			pScan->pIdxExpr = pIdx->aColExpr->a[j].pExpr;
-		} else if (iColumn >= 0) {
+		if (iColumn >= 0) {
 			char affinity =
 				pIdx->pTable->def->fields[iColumn].affinity;
 			pScan->idxaff = affinity;
@@ -387,8 +380,6 @@ whereScanInit(WhereScan * pScan,	/* The WhereScan object being initialized */
 			pScan->coll = sql_index_collation(pIdx, j, &id);
 			pScan->is_column_seen = true;
 		}
-	} else if (iColumn == XN_EXPR) {
-		return 0;
 	}
 	pScan->opMask = opMask;
 	pScan->k = 0;
@@ -2713,7 +2704,6 @@ indexMightHelpWithOrderBy(WhereLoopBuilder * pBuilder,
 			  Index * pIndex, int iCursor)
 {
 	ExprList *pOB;
-	ExprList *aColExpr;
 	int ii, jj;
 	int nIdxCol = index_column_count(pIndex);
 	if (index_is_unordered(pIndex))
@@ -2729,16 +2719,6 @@ indexMightHelpWithOrderBy(WhereLoopBuilder * pBuilder,
 				if (pExpr->iColumn == pIndex->aiColumn[jj])
 					return 1;
 			}
-		} else if ((aColExpr = pIndex->aColExpr) != 0) {
-			for (jj = 0; jj < nIdxCol; jj++) {
-				if (pIndex->aiColumn[jj] != XN_EXPR)
-					continue;
-				if (sqlite3ExprCompare
-				    (pExpr, aColExpr->a[jj].pExpr,
-				     iCursor) == 0) {
-					return 1;
-				}
-			}
 		}
 	}
 	return 0;
@@ -3429,10 +3409,7 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo,	/* The WHERE clause */
 						if (pOBExpr->iColumn != iColumn)
 							continue;
 					} else {
-						if (sqlite3ExprCompare(pOBExpr,
-								       pIndex->aColExpr->a[j].pExpr, iCur)) {
-							continue;
-						}
+						continue;
 					}
 					if (iColumn >= 0) {
 						bool is_found;
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 09b2671..bf2a2a2 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -48,8 +48,6 @@ static const char *
 explainIndexColumnName(Index * pIdx, int i)
 {
 	i = pIdx->aiColumn[i];
-	if (i == XN_EXPR)
-		return "<expr>";
 	return pIdx->pTable->def->fields[i].name;
 }
 
@@ -719,7 +717,6 @@ codeAllEqualityTerms(Parse * pParse,	/* Parsing context */
 		for (j = 0; j < nSkip; j++) {
 			sqlite3VdbeAddOp3(v, OP_Column, iIdxCur,
 					  pIdx->aiColumn[j], regBase + j);
-			testcase(pIdx->aiColumn[j] == XN_EXPR);
 			VdbeComment((v, "%s", explainIndexColumnName(pIdx, j)));
 		}
 	}
@@ -1259,8 +1256,8 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
 			 * FYI: entries in an index are ordered as follows:
 			 *      NULL, ... NULL, min_value, ...
 			 */
-			if ((j >= 0 && pIdx->pTable->def->fields[j].is_nullable)
-			    || j == XN_EXPR) {
+			if (j >= 0
+			    && pIdx->pTable->def->fields[j].is_nullable) {
 				assert(pLoop->nSkip == 0);
 				bSeekPastNull = 1;
 				nExtraReg = 1;
@@ -1305,11 +1302,9 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about t
 #endif
 			if (pRangeStart == 0) {
 				j = pIdx->aiColumn[nEq];
-				if ((j >= 0 &&
-				     pIdx->pTable->def->fields[j].is_nullable)||
-				    j == XN_EXPR) {
+				if (j >= 0 &&
+				    pIdx->pTable->def->fields[j].is_nullable)
 					bSeekPastNull = 1;
-				}
 			}
 		}
 		assert(pRangeEnd == 0
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index aa6d452..e5d6b44 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -887,26 +887,19 @@ exprSelectUsage(WhereMaskSet * pMaskSet, Select * pS)
  * in any index.  Return TRUE (1) if pExpr is an indexed term and return
  * FALSE (0) if not.  If TRUE is returned, also set *piCur to the cursor
  * number of the table that is indexed and *piColumn to the column number
- * of the column that is indexed, or XN_EXPR (-2) if an expression is being
- * indexed.
+ * of the column that is indexed.
  *
  * If pExpr is a TK_COLUMN column reference, then this routine always returns
  * true even if that particular column is not indexed, because the column
  * might be added to an automatic index later.
  */
 static int
-exprMightBeIndexed(SrcList * pFrom,	/* The FROM clause */
-		   int op,	/* The specific comparison operator */
-		   Bitmask mPrereq,	/* Bitmask of FROM clause terms referenced by pExpr */
+exprMightBeIndexed(int op,	/* The specific comparison operator */
 		   Expr * pExpr,	/* An operand of a comparison operator */
 		   int *piCur,	/* Write the referenced table cursor number here */
 		   int *piColumn	/* Write the referenced table column number here */
     )
 {
-	Index *pIdx;
-	int i;
-	int iCur;
-
 	/* If this expression is a vector to the left or right of a
 	 * inequality constraint (>, <, >= or <=), perform the processing
 	 * on the first element of the vector.
@@ -923,27 +916,6 @@ exprMightBeIndexed(SrcList * pFrom,	/* The FROM clause */
 		*piColumn = pExpr->iColumn;
 		return 1;
 	}
-	if (mPrereq == 0)
-		return 0;	/* No table references */
-	if ((mPrereq & (mPrereq - 1)) != 0)
-		return 0;	/* Refs more than one table */
-	for (i = 0; mPrereq > 1; i++, mPrereq >>= 1) {
-	}
-	iCur = pFrom->a[i].iCursor;
-	for (pIdx = pFrom->a[i].pTab->pIndex; pIdx; pIdx = pIdx->pNext) {
-		if (pIdx->aColExpr == 0)
-			continue;
-		for (i = 0; i < pIdx->nColumn; i++) {
-			if (pIdx->aiColumn[i] != XN_EXPR)
-				continue;
-			if (sqlite3ExprCompare
-			    (pExpr, pIdx->aColExpr->a[i].pExpr, iCur) == 0) {
-				*piCur = iCur;
-				*piColumn = XN_EXPR;
-				return 1;
-			}
-		}
-	}
 	return 0;
 }
 
@@ -1038,13 +1010,13 @@ exprAnalyze(SrcList * pSrc,	/* the FROM clause */
 		}
 
 		if (exprMightBeIndexed
-		    (pSrc, op, prereqLeft, pLeft, &iCur, &iColumn)) {
+		    (op, pLeft, &iCur, &iColumn)) {
 			pTerm->leftCursor = iCur;
 			pTerm->u.leftColumn = iColumn;
 			pTerm->eOperator = operatorMask(op) & opMask;
 		}
 		if (pRight
-		    && exprMightBeIndexed(pSrc, op, pTerm->prereqRight, pRight,
+		    && exprMightBeIndexed(op, pRight,
 					  &iCur, &iColumn)
 		    ) {
 			WhereTerm *pNew;
-- 
2.16.2

  parent reply	other threads:[~2018-06-01 15:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-01 15:16 [tarantool-patches] [PATCH 0/3] sql: implement point where for DELETE stmts Kirill Yukhin
2018-06-01 15:16 ` [tarantool-patches] [PATCH 1/3] sql: fetch primary index for affinity only Kirill Yukhin
2018-06-01 18:00   ` [tarantool-patches] " Vladislav Shpilevoy
2018-06-07 12:03     ` Kirill Yukhin
2018-06-07 15:01       ` Vladislav Shpilevoy
2018-06-08  8:25         ` Kirill Yukhin
2018-06-01 15:16 ` Kirill Yukhin [this message]
2018-06-01 18:00   ` [tarantool-patches] Re: [PATCH 2/3] sql: remove expressions from SQL indexes Vladislav Shpilevoy
2018-06-01 15:16 ` [tarantool-patches] [PATCH 3/3] sql: implement point where for DELETE stmts Kirill Yukhin
2018-06-01 18:00   ` [tarantool-patches] " Vladislav Shpilevoy
2018-06-18  2:11     ` n.pettik
2018-06-18 10:44       ` Vladislav Shpilevoy
2018-06-18 10:51         ` Vladislav Shpilevoy
2018-06-18 12:29         ` n.pettik
2018-06-18 12:40           ` Vladislav Shpilevoy
2018-06-18 14:00             ` n.pettik
2018-06-18 14:17               ` Vladislav Shpilevoy
2018-06-19  8:03                 ` Kirill Yukhin
2018-06-14 12:41 ` [tarantool-patches] Re: [PATCH 0/3] " Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=787e55ddaaeeec54e870d3f493fc235fe64ffd51.1527865931.git.kyukhin@tarantool.org \
    --to=kyukhin@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] [PATCH 2/3] sql: remove expressions from SQL indexes' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox