From 758ab1a4218b23ddc7b8809523e486ee1a92cb17 Mon Sep 17 00:00:00 2001
From: Nikita Pettik <korablev@tarantool.org>
Date: Tue, 18 Dec 2018 15:44:06 +0300
Subject: [PATCH] sql: remove numeric affinity
Numeric affinity in SQLite means the same as real, except that it
forces integer values into floating point representation in case
it can be converted without loss (e.g. 2.0 -> 2).
Since in Tarantool core there is no difference between numeric and real
values (both are stored as values of Tarantool type NUMBER), lets
remove numeric affinity and use instead real.
The only real pitfall is implicit conversion mentioned above. We can't
pass *.0 as an iterator value since our fast comparators (TupleCompare,
TupleCompareWithKey) are designed to work with only values of same MP_
type. They do not use slow tuple_compare_field() which is able to
compare double and integer. Solution to this problem is simple: lets
always attempt at encoding floats as ints if conversion takes place
without loss. This is a straightforward approach, but to implement it we
need to care about reversed (decoding) situation.
OP_Column fetches from msgpack field with given number and stores it as
a native VDBE memory object. Type of that memory is based on type of
msgpack value. So, if space field is of type NUMBER and holds value 1,
type of VDBE memory will be INT (after decoding), not float 1.0. As a
result, further calculations may be wrong: for instance, instead of
floating point division, we could get integer division. To cope with
this problem, lets add auxiliary conversion to decoding routine which
uses space format of tuple to be decoded. It is worth mentioning that
ephemeral spaces don't feature space format, so we are going to rely on
type of key parts. Finally, internal VDBE merge sorter also operates on
entries encoded into msgpack. To fix this case, we check type of
ORDER BY/GROUP BY arguments: if they are of type float, we are emitting
additional opcode OP_AffinityReal to force float type after encoding.
Part of #3698
---
src/box/field_def.h | 1 -
src/box/lua/lua_sql.c | 2 +-
src/box/sql.c | 9 ++++++---
src/box/sql/build.c | 1 -
src/box/sql/expr.c | 21 ++++++++++++---------
src/box/sql/select.c | 10 +++++++---
src/box/sql/sqliteInt.h | 2 +-
src/box/sql/vdbe.c | 26 ++++++++++++++++++--------
src/box/sql/vdbemem.c | 24 +++++++++++++++++-------
test/sql-tap/tkt-80e031a00f.test.lua | 12 ++++++------
10 files changed, 68 insertions(+), 40 deletions(-)
diff --git a/src/box/field_def.h b/src/box/field_def.h
index bd3f9ba455..93e38ea552 100644
--- a/src/box/field_def.h
+++ b/src/box/field_def.h
@@ -77,7 +77,6 @@ enum affinity_type {
AFFINITY_UNDEFINED = 0,
AFFINITY_BLOB = 'A',
AFFINITY_TEXT = 'B',
- AFFINITY_NUMERIC = 'C',
AFFINITY_INTEGER = 'D',
AFFINITY_REAL = 'E',
};
diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
index 5c77b80367..ab0b7f37cb 100644
--- a/src/box/lua/lua_sql.c
+++ b/src/box/lua/lua_sql.c
@@ -158,7 +158,7 @@ lbox_sql_create_function(struct lua_State *L)
else if (strcmp(type_arg, "FLOAT") == 0)
type = AFFINITY_REAL;
else if (strcmp(type_arg, "NUM") == 0)
- type = AFFINITY_NUMERIC;
+ type = AFFINITY_REAL;
else if (strcmp(type_arg, "BLOB") == 0)
type = AFFINITY_BLOB;
else
diff --git a/src/box/sql.c b/src/box/sql.c
index 45d15bb465..baa67da397 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -382,15 +382,18 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info)
for (uint32_t i = 0; i < field_count; ++i) {
struct key_part_def *part = &ephemer_key_parts[i];
part->fieldno = i;
- part->type = FIELD_TYPE_SCALAR;
part->nullable_action = ON_CONFLICT_ACTION_NONE;
part->is_nullable = true;
part->sort_order = SORT_ORDER_ASC;
part->path = NULL;
- if (def != NULL && i < def->part_count)
+ if (def != NULL && i < def->part_count) {
+ assert(def->parts[i].type < field_type_MAX);
+ part->type = def->parts[i].type;
part->coll_id = def->parts[i].coll_id;
- else
+ } else {
part->coll_id = COLL_NONE;
+ part->type = FIELD_TYPE_SCALAR;
+ }
}
struct key_def *ephemer_key_def = key_def_new(ephemer_key_parts,
field_count);
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index b8b91657db..63d3727893 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -492,7 +492,6 @@ sql_affinity_to_field_type(enum affinity_type affinity)
case AFFINITY_INTEGER:
return FIELD_TYPE_INTEGER;
case AFFINITY_REAL:
- case AFFINITY_NUMERIC:
return FIELD_TYPE_NUMBER;
case AFFINITY_TEXT:
return FIELD_TYPE_STRING;
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index d83be51015..a6cf7bdb13 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -261,7 +261,7 @@ sql_affinity_result(enum affinity_type aff1, enum affinity_type aff2)
*/
if (sqlite3IsNumericAffinity(aff1)
|| sqlite3IsNumericAffinity(aff2)) {
- return AFFINITY_NUMERIC;
+ return AFFINITY_REAL;
} else {
return AFFINITY_BLOB;
}
@@ -2164,12 +2164,10 @@ sqlite3ExprNeedsNoAffinityChange(const Expr * p, char aff)
op = p->op2;
switch (op) {
case TK_INTEGER:{
- return aff == AFFINITY_INTEGER
- || aff == AFFINITY_NUMERIC;
+ return aff == AFFINITY_INTEGER;
}
case TK_FLOAT:{
- return aff == AFFINITY_REAL
- || aff == AFFINITY_NUMERIC;
+ return aff == AFFINITY_REAL;
}
case TK_STRING:{
return aff == AFFINITY_TEXT;
@@ -2181,7 +2179,7 @@ sqlite3ExprNeedsNoAffinityChange(const Expr * p, char aff)
assert(p->iTable >= 0); /* p cannot be part of a CHECK constraint */
return p->iColumn < 0
&& (aff == AFFINITY_INTEGER
- || aff == AFFINITY_NUMERIC);
+ || aff == AFFINITY_REAL);
}
default:{
return 0;
@@ -3696,6 +3694,11 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
sqlite3VdbeAddOp3(v, OP_Column,
pAggInfo->sortingIdxPTab,
pCol->iSorterColumn, target);
+ if (pCol->space_def->fields[pExpr->iAgg].type ==
+ FIELD_TYPE_NUMBER) {
+ sqlite3VdbeAddOp1(v, OP_RealAffinity,
+ target);
+ }
return target;
}
/* Otherwise, fall thru into the TK_COLUMN case */
@@ -3881,14 +3884,14 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
testcase(regFree1 == 0);
testcase(regFree2 == 0);
if (op != TK_CONCAT)
- pExpr->affinity = AFFINITY_NUMERIC;
+ pExpr->affinity = AFFINITY_REAL;
else
pExpr->affinity = AFFINITY_TEXT;
break;
}
case TK_UMINUS:{
Expr *pLeft = pExpr->pLeft;
- pExpr->affinity = AFFINITY_NUMERIC;
+ pExpr->affinity = AFFINITY_REAL;
assert(pLeft);
if (pLeft->op == TK_INTEGER) {
expr_code_int(pParse, pLeft, true, target);
@@ -4180,7 +4183,7 @@ sqlite3ExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
target);
}
case TK_UPLUS:{
- pExpr->affinity = AFFINITY_NUMERIC;
+ pExpr->affinity = AFFINITY_REAL;
return sqlite3ExprCodeTarget(pParse, pExpr->pLeft,
target);
}
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index dfe9a83bca..e063e98909 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1428,6 +1428,8 @@ sql_expr_list_to_key_info(struct Parse *parse, struct ExprList *list, int start)
sql_expr_coll(parse, item->pExpr, &unused, &id);
part->coll_id = id;
part->sort_order = item->sort_order;
+ enum affinity_type aff = sqlite3ExprAffinity(item->pExpr);
+ part->type = sql_affinity_to_field_type(aff);
}
return key_info;
}
@@ -1724,7 +1726,6 @@ generateColumnNames(Parse * pParse, /* Parser context */
SQLITE_TRANSIENT);
break;
case AFFINITY_REAL:
- case AFFINITY_NUMERIC:
sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, "NUMERIC",
SQLITE_TRANSIENT);
break;
@@ -5817,8 +5818,11 @@ sqlite3Select(Parse * pParse, /* The parser context */
/* If the output is destined for a temporary table, open that table.
*/
if (pDest->eDest == SRT_EphemTab) {
- sqlite3VdbeAddOp2(v, OP_OpenTEphemeral, pDest->reg_eph,
- pEList->nExpr + 1);
+ struct sql_key_info *key_info =
+ sql_expr_list_to_key_info(pParse, pEList, 0);
+ sqlite3VdbeAddOp4(v, OP_OpenTEphemeral, pDest->reg_eph,
+ pEList->nExpr + 1, 0, (char *)key_info,
+ P4_KEYINFO);
sqlite3VdbeAddOp3(v, OP_IteratorOpen, pDest->iSDParm, 0,
pDest->reg_eph);
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index de12769698..29e1657aea 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1788,7 +1788,7 @@ struct Savepoint {
#define SAVEPOINT_RELEASE 1
#define SAVEPOINT_ROLLBACK 2
-#define sqlite3IsNumericAffinity(X) ((X)>=AFFINITY_NUMERIC)
+#define sqlite3IsNumericAffinity(X) ((X)>=AFFINITY_INTEGER)
/*
* The AFFINITY_MASK values masks off the significant bits of an
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 9fc362f0a3..15c3305e38 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -311,7 +311,6 @@ applyNumericAffinity(Mem *pRec, int bTryForInt)
*
* AFFINITY_INTEGER:
* AFFINITY_REAL:
- * AFFINITY_NUMERIC:
* Try to convert mem to an integer representation or a
* floating-point representation if an integer representation
* is not possible. Note that the integer representation is
@@ -346,13 +345,9 @@ mem_apply_affinity(struct Mem *record, enum affinity_type affinity)
}
return sqlite3VdbeMemIntegerify(record, false);
case AFFINITY_REAL:
- if ((record->flags & MEM_Real) == MEM_Real)
- return 0;
- return sqlite3VdbeMemRealify(record);
- case AFFINITY_NUMERIC:
if ((record->flags & (MEM_Real | MEM_Int)) != 0)
return 0;
- return sqlite3VdbeMemNumerify(record);
+ return sqlite3VdbeMemRealify(record);
case AFFINITY_TEXT:
/*
* Only attempt the conversion to TEXT if there is
@@ -2006,7 +2001,6 @@ case OP_Cast: { /* in1 */
assert(pOp->p2>=AFFINITY_BLOB && pOp->p2<=AFFINITY_REAL);
testcase( pOp->p2==AFFINITY_TEXT);
testcase( pOp->p2==AFFINITY_BLOB);
- testcase( pOp->p2==AFFINITY_NUMERIC);
testcase( pOp->p2==AFFINITY_INTEGER);
testcase( pOp->p2==AFFINITY_REAL);
pIn1 = &aMem[pOp->p1];
@@ -2174,7 +2168,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
} else {
/* Neither operand is NULL. Do a comparison. */
affinity = pOp->p5 & AFFINITY_MASK;
- if (affinity>=AFFINITY_NUMERIC) {
+ if (affinity>=AFFINITY_INTEGER) {
if ((flags1 | flags3)&MEM_Str) {
if ((flags1 & (MEM_Int|MEM_Real|MEM_Str))==MEM_Str) {
applyNumericAffinity(pIn1,0);
@@ -2769,6 +2763,22 @@ case OP_Column: {
pDest->flags = MEM_Blob|MEM_Ephem|MEM_Subtype;
pDest->subtype = SQL_SUBTYPE_MSGPACK;
}
+ if ((pDest->flags & MEM_Int) != 0 &&
+ pC->eCurType == CURTYPE_TARANTOOL) {
+ enum field_type f = FIELD_TYPE_ANY;
+ /*
+ * Ephemeral spaces feature only one index
+ * covering all fields, but ephemeral spaces
+ * lack format. So, we can fetch type from
+ * key parts.
+ */
+ if (pC->uc.pCursor->curFlags & BTCF_TEphemCursor)
+ f = pC->uc.pCursor->index->def->key_def->parts[p2].type;
+ else if (pC->uc.pCursor->curFlags & BTCF_TaCursor)
+ f = pC->uc.pCursor->space->def->fields[p2].type;
+ if (f == FIELD_TYPE_NUMBER)
+ sqlite3VdbeMemSetDouble(pDest, pDest->u.i);
+ }
/*
* Add 0 termination (at most for strings)
* Not sure why do we check MEM_Ephem
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 7b21f2aaa4..754dc207bc 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -604,8 +604,7 @@ sqlite3VdbeMemCast(Mem * pMem, u8 aff)
{
if (pMem->flags & MEM_Null)
return SQLITE_OK;
- if ((pMem->flags & MEM_Blob) != 0 &&
- (aff == AFFINITY_REAL || aff == AFFINITY_NUMERIC)) {
+ if ((pMem->flags & MEM_Blob) != 0 && aff == AFFINITY_REAL) {
if (sql_atoi64(pMem->z, (int64_t *) &pMem->u.i, pMem->n) == 0) {
MemSetTypeFlag(pMem, MEM_Real);
pMem->u.r = pMem->u.i;
@@ -628,8 +627,6 @@ sqlite3VdbeMemCast(Mem * pMem, u8 aff)
return 0;
}
return SQLITE_ERROR;
- case AFFINITY_NUMERIC:
- return sqlite3VdbeMemNumerify(pMem);
case AFFINITY_INTEGER:
if ((pMem->flags & MEM_Blob) != 0) {
if (sql_atoi64(pMem->z, (int64_t *) &pMem->u.i,
@@ -1321,7 +1318,7 @@ valueFromExpr(sqlite3 * db, /* The database connection */
}
if ((op == TK_INTEGER || op == TK_FLOAT)
&& affinity == AFFINITY_BLOB) {
- sqlite3ValueApplyAffinity(pVal, AFFINITY_NUMERIC);
+ sqlite3ValueApplyAffinity(pVal, AFFINITY_REAL);
} else {
sqlite3ValueApplyAffinity(pVal, affinity);
}
@@ -1721,15 +1718,28 @@ void
mpstream_encode_vdbe_mem(struct mpstream *stream, struct Mem *var)
{
assert(memIsValid(var));
+ int64_t i;
if (var->flags & MEM_Null) {
mpstream_encode_nil(stream);
} else if (var->flags & MEM_Real) {
+ /*
+ * We can't pass to INT iterator float
+ * value. Hence, if floating point value
+ * lacks fractional component, we can
+ * encode it as INT and successfully
+ * pass to INT iterator.
+ */
+ i = var->u.r;
+ if (i == var->u.r)
+ goto encode_int;
mpstream_encode_double(stream, var->u.r);
} else if (var->flags & MEM_Int) {
+ i = var->u.i;
+encode_int:
if (var->u.i >= 0)
- mpstream_encode_uint(stream, var->u.i);
+ mpstream_encode_uint(stream, i);
else
- mpstream_encode_int(stream, var->u.i);
+ mpstream_encode_int(stream, i);
} else if (var->flags & MEM_Str) {
mpstream_encode_strn(stream, var->z, var->n);
} else if (var->flags & MEM_Bool) {
diff --git a/test/sql-tap/tkt-80e031a00f.test.lua b/test/sql-tap/tkt-80e031a00f.test.lua
index 0cbdb47a2b..2d4f81798c 100755
--- a/test/sql-tap/tkt-80e031a00f.test.lua
+++ b/test/sql-tap/tkt-80e031a00f.test.lua
@@ -346,7 +346,7 @@ test:do_catchsql_test(
SELECT 'hello' IN t1
]], {
-- <tkt-80e031a00f.27>
- 1, 'Type mismatch: can not convert hello to numeric'
+ 1, 'Type mismatch: can not convert hello to real'
-- </tkt-80e031a00f.27>
})
@@ -356,7 +356,7 @@ test:do_catchsql_test(
SELECT 'hello' NOT IN t1
]], {
-- <tkt-80e031a00f.28>
- 1, 'Type mismatch: can not convert hello to numeric'
+ 1, 'Type mismatch: can not convert hello to real'
-- </tkt-80e031a00f.28>
})
@@ -380,23 +380,23 @@ test:do_execsql_test(
-- </tkt-80e031a00f.30>
})
-test:do_execsql_test(
+test:do_catchsql_test(
"tkt-80e031a00f.31",
[[
SELECT x'303132' IN t1
]], {
-- <tkt-80e031a00f.31>
- 0
+ 1, 'Type mismatch: can not convert 012 to real'
-- </tkt-80e031a00f.31>
})
-test:do_execsql_test(
+test:do_catchsql_test(
"tkt-80e031a00f.32",
[[
SELECT x'303132' NOT IN t1
]], {
-- <tkt-80e031a00f.32>
- 1
+ 1, 'Type mismatch: can not convert 012 to real'
-- </tkt-80e031a00f.32>
})
--
GitLab