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 B1E9A23CE9 for ; Mon, 14 May 2018 07:20:31 -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 1g67BLgIG-Ru for ; Mon, 14 May 2018 07:20:31 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 4584721022 for ; Mon, 14 May 2018 07:20:31 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v5 3/3] sql: space_def* instead of Table* in Expr References: <3be5bcd1-a445-d39c-8da9-a50799bf7693@tarantool.org> From: Kirill Shcherbatov Message-ID: <1c238639-cddc-10c8-6f2d-6b24861e3f33@tarantool.org> Date: Mon, 14 May 2018 14:20:27 +0300 MIME-Version: 1.0 In-Reply-To: <3be5bcd1-a445-d39c-8da9-a50799bf7693@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: "v.shpilevoy@tarantool.org" > 1. Please, do not use SQLITE prefix in Tarantool. Lets name it with > AFFINITY prefix instead of SQLITE_AFF. enum affinity_type { - SQLITE_AFF_UNDEFINED = 0, - SQLITE_AFF_BLOB = 'A', - SQLITE_AFF_TEXT = 'B', - SQLITE_AFF_NUMERIC = 'C', - SQLITE_AFF_INTEGER = 'D', - SQLITE_AFF_REAL = 'E', + AFFINITY_UNDEFINED = 0, + AFFINITY_BLOB = 'A', + AFFINITY_TEXT = 'B', + AFFINITY_NUMERIC = 'C', + AFFINITY_INTEGER = 'D', + AFFINITY_REAL = 'E', }; > 2. Fix the comment as well> 3. Now this function is useless. You always can get the collation from > coll_by_id(def->fields[column].coll_id). diff --git a/src/box/sql/build.c b/src/box/sql/build.c index f2d325c..33815b8 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1032,7 +1032,7 @@ sqlite3AddCollateType(Parse * pParse, Token * pToken) assert(pIdx->nColumn == 1); if (pIdx->aiColumn[0] == i) { pIdx->coll_array[0] = - sql_column_collation(p->def, i); + coll_by_id(p->def->fields[i].coll_id); } } } @@ -1040,39 +1040,6 @@ sqlite3AddCollateType(Parse * pParse, Token * pToken) } /** - * Return collation of given column from space_def . - * - * @param table Table which is used to fetch column. - * @param column Number of column. - * @retval Pointer to collation. - */ -struct coll * -sql_column_collation(struct space_def *def, uint32_t column) -{ - assert(def != NULL); - uint32_t space_id = def->id; - struct space *space = space_by_id(space_id); - /* - * It is not always possible to fetch collation directly - * from struct space. To be more precise when: - * 1. space is ephemeral. Thus, its id is zero and - * it can't be found in space cache. - * 2. space is a view. Hence, it lacks any functional - * parts such as indexes or fields. - * 3. space is under construction. So, the same as p.1 - * it can't be found in space cache. - * In cases mentioned above collation is fetched from - * SQL specific structures. - */ - if (space == NULL || space_index(space, 0) == NULL) { - assert(column < (uint32_t)def->field_count); - return coll_by_id(def->fields[column].coll_id); - } - - return space->format->fields[column].coll; -} - -/** * Return name of given column collation from index. * * @param idx Index which is used to fetch column. @@ -3107,7 +3074,7 @@ sqlite3CreateIndex(Parse * pParse, /* All information about this parse */ goto exit_create_index; } } else if (j >= 0) { - coll = sql_column_collation(pTab->def, j); + coll = coll_by_id(pTab->def->fields[j].coll_id); } else { coll = NULL; } diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 51c50e0..59f239d 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -34,6 +34,7 @@ * for generating VDBE code that evaluates expressions in SQLite. */ #include +#include "box/coll_cache.h" #include "sqliteInt.h" #include "box/session.h" @@ -187,7 +188,8 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_found) */ int j = p->iColumn; if (j >= 0) { - coll = sql_column_collation(p->space_def, j); + coll = coll_by_id( + p->space_def->fields[j].coll_id); *is_found = true; } break; diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c index 7c83924..c7b1cda 100644 --- a/src/box/sql/fkey.c +++ b/src/box/sql/fkey.c @@ -34,6 +34,7 @@ * support to compiled SQL statements. */ #include +#include "box/coll_cache.h" #include "sqliteInt.h" #include "box/session.h" #include "tarantoolInt.h" @@ -298,8 +299,8 @@ sqlite3FkLocateIndex(Parse * pParse, /* Parse context to store any error in */ * unusable. Bail out early in this case. */ struct coll *def_coll; - def_coll = sql_column_collation(pParent->def, - iCol); + def_coll = coll_by_id( + pParent->def->fields[iCol].coll_id); struct coll *coll; coll = sql_index_collation(pIdx, i); if (def_coll != coll) diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index b388735..710a388 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -37,6 +37,7 @@ #include "tarantoolInt.h" #include "box/session.h" #include "box/schema.h" +#include "box/coll_cache.h" #include "bit/bit.h" /* @@ -1809,8 +1810,8 @@ xferOptimization(Parse * pParse, /* Parser context */ * Collating sequence must be the same on all * columns. */ - if (sql_column_collation(pDest->def, i) != - sql_column_collation(pSrc->def, i)) + if (coll_by_id(pDest->def->fields[i].coll_id) != + coll_by_id(pSrc->def->fields[i].coll_id)) return 0; /* The tab2 must be NOT NULL if tab1 is */ if (!space_def_column_is_nullable(pDest->def, i) diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 02489b7..7cca3d5 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -3499,8 +3499,6 @@ void sqlite3AddCollateType(Parse *, Token *); const char * column_collation_name(Table *, uint32_t); -struct coll * -sql_column_collation(struct space_def *, uint32_t); const char * index_collation_name(Index *, uint32_t); struct coll * > 4. Lets do not inline affinity values: use AFFINITY_INTEGER. @@ -3184,7 +3184,7 @@ sqlite3ExprCodeIN(Parse * pParse, /* Parsing and code generating context */ - && affinity == 'D' + && affinity == AFFINITY_INTEGER @@ -1303,7 +1303,8 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */ - if (pTab->zColAff[pIdx->aiColumn[0]] == 'D') { + if (pTab->zColAff[pIdx->aiColumn[0]] == + AFFINITY_INTEGER) { @@ -1392,7 +1392,7 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about t - if (nPkCol == 1 && affinity == 'D') { + if (nPkCol == 1 && affinity == AFFINITY_INTEGER) { > 8. For flags use 'is_' prefix, please. I know, it is not your code, but if > you modify it, then lets do it correctly.@@ -373,7 +373,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ - bool nullable = + bool is_nullable = > 9. Please, use /** prefix for comments out of function body. > 10. Same. - /* Pointer to space definition. */ + /** Pointer to space definition. */ - /* Pointer for table relative definition. */ + /** Pointer for table relative definition. */ > 12. Perfect. Now this function is useless. It can be inlined and removed. diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 9057301..9a8f045 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -2139,8 +2139,7 @@ sqlite3ExprCanBeNull(const Expr * p) assert(p->space_def != NULL); return ExprHasProperty(p, EP_CanBeNull) || (p->iColumn >= 0 - && space_def_column_is_nullable(p->space_def, - p->iColumn)); + && p->space_def->fields[p->iColumn].is_nullable); default: return 1; } diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 71567cf..1a883e2 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -1120,7 +1120,7 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */ /* Don't bother checking for NOT NULL on columns that do not change */ continue; } - if (space_def_column_is_nullable(pTab->def, i) + if (pTab->def->fields[i].is_nullable || (pTab->tabFlags & TF_Autoincrement && pTab->iAutoIncPKey == i)) continue; /* This column is allowed to be NULL */ @@ -1815,8 +1815,8 @@ xferOptimization(Parse * pParse, /* Parser context */ coll_by_id(pSrc->def->fields[i].coll_id)) return 0; /* The tab2 must be NOT NULL if tab1 is */ - if (!space_def_column_is_nullable(pDest->def, i) - && space_def_column_is_nullable(pSrc->def, i)) + if (!pDest->def->fields[i].is_nullable + && pSrc->def->fields[i].is_nullable) return 0; /* Default values for second and subsequent columns need to match. */ if (i > 0) { diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index 4d7b834..c5ae5cb 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -374,8 +374,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ } } bool is_nullable = - space_def_column_is_nullable( - pTab->def, i); + pTab->def->fields[i]. + is_nullable; uint32_t space_id = SQLITE_PAGENO_TO_SPACEID( pTab->tnum); diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 55a1a87..75699ae 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -4117,9 +4117,6 @@ extern int sqlite3InitDatabase(sqlite3 * db); enum on_conflict_action table_column_nullable_action(struct Table *tab, uint32_t column); -bool -space_def_column_is_nullable(struct space_def *def, uint32_t column); - /** * Initialize a new parser object. * @param parser object to initialize. diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 807e017..4e3b236 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -4712,22 +4712,3 @@ table_column_nullable_action(struct Table *tab, uint32_t column) return field.nullable_action; } - -/** - * Return nullable flag value of given column in given table. - * FIXME: This is implemented in expensive way. For each invocation table lookup - * is performed. In future, first param will be replaced with pointer to struct - * space. - * - * @param tab pointer to the table - * @param column column number for which action to be returned - * @return return nullability flag value - */ -bool -space_def_column_is_nullable(struct space_def *def, uint32_t column) -{ - assert(def->fields[column].is_nullable == - nullable_action_is_nullable( - def->fields[column].nullable_action)); - return def->fields[column].is_nullable; -} diff --git a/src/box/sql/where.c b/src/box/sql/where.c index 8e77e3e..f3a10c0 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -493,7 +493,7 @@ indexColumnNotNull(Index * pIdx, int iCol) assert(iCol >= 0 && iCol < (int)index_column_count(pIdx)); j = pIdx->aiColumn[iCol]; if (j >= 0) { - return !space_def_column_is_nullable(pIdx->pTable->def, j); + return !pIdx->pTable->def->fields[j].is_nullable; } else if (j == (-1)) { return 1; } else { @@ -3333,8 +3333,8 @@ wherePathSatisfiesOrderBy(WhereInfo * pWInfo, /* The WHERE clause */ if (isOrderDistinct && iColumn >= 0 && j >= pLoop->nEq - && space_def_column_is_nullable( - pIndex->pTable->def, iColumn)) { + && pIndex->pTable->def->fields[iColumn]. + is_nullable) { isOrderDistinct = 0; } diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c index 0bc6b39..a3db23b 100644 --- a/src/box/sql/wherecode.c +++ b/src/box/sql/wherecode.c @@ -1261,8 +1261,7 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about t * NULL, ... NULL, min_value, ... */ if ((j >= 0 && - space_def_column_is_nullable(pIdx->pTable->def, - j)) || + pIdx->pTable->def->fields[j].is_nullable) || j == XN_EXPR) { assert(pLoop->nSkip == 0); bSeekPastNull = 1; @@ -1308,9 +1307,8 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about t #endif if (pRangeStart == 0) { j = pIdx->aiColumn[nEq]; - if ((j >= 0 - && space_def_column_is_nullable( - pIdx->pTable->def, j)) || + if ((j >= 0 && + pIdx->pTable->def->fields[j].is_nullable) || j == XN_EXPR) { bSeekPastNull = 1; }