[tarantool-patches] Re: [PATCH v5 3/3] sql: space_def* instead of Table* in Expr
Kirill Shcherbatov
kshcherbatov at tarantool.org
Mon May 14 14:20:27 MSK 2018
> 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 <box/coll.h>
+#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 <box/coll.h>
+#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;
}
More information about the Tarantool-patches
mailing list