From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id ED15F290DD for ; Fri, 1 Jun 2018 11:16:19 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id DFcA7ymMoZYs for ; Fri, 1 Jun 2018 11:16:19 -0400 (EDT) Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 64B19290D6 for ; Fri, 1 Jun 2018 11:16:19 -0400 (EDT) From: Kirill Yukhin Subject: [tarantool-patches] [PATCH 2/3] sql: remove expressions from SQL indexes Date: Fri, 1 Jun 2018 18:16:11 +0300 Message-Id: <787e55ddaaeeec54e870d3f493fc235fe64ffd51.1527865931.git.kyukhin@tarantool.org> In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: v.shpilevoy@tarantool.org Cc: tarantool-patches@freelists.org, Kirill Yukhin 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 ""; 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