[tarantool-patches] [PATCH 1/2] sql: extend struct Mem with field_type member
Nikita Pettik
korablev at tarantool.org
Sat Jul 27 21:45:35 MSK 2019
There's several reasons to do so. First one is to improve operability of
built-in function 'typeof()' which returns string containing argument's
type. Using only format (msgpack) type we can't determine real field
type of null values. Moreover, result of CAST should feature the same
type as it was specified in operation. For instance:
typeof(CAST(0 AS INTEGER)) should return "integer", not "unsigned".
The second reason is different rules for comparison involving values of
SCALAR type. In Tarantool NoSQL it is allowed to compare values of
"incompatible" types like booleans and strings (using heuristics which
says that values of one type always are greater/less that values of
another type), in case they are stored in SCALAR field type. Without
storing actual field type in struct Mem it is obvious impossible to
achieve.
Thus, now field_type member in struct Mem represents supposed field type
in case value is fetched from tuple or is a result of CAST operation.
Closes #4148
---
src/box/sql/func.c | 11 +++++++++++
src/box/sql/vdbe.c | 36 +++++++++++++++++++++++++++++++++++-
src/box/sql/vdbeInt.h | 11 +++++++++++
src/box/sql/vdbeapi.c | 1 +
src/box/sql/vdbeaux.c | 1 +
src/box/sql/vdbemem.c | 3 +++
src/box/sql/vdbesort.c | 6 ++++++
test/sql-tap/cast.test.lua | 8 ++++----
test/sql-tap/table.test.lua | 4 ++--
test/sql/types.result | 35 ++++++++++++++++++++++++++++++++++-
test/sql/types.test.lua | 11 +++++++++++
11 files changed, 119 insertions(+), 8 deletions(-)
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index fcf147c96..f50df105d 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -107,6 +107,17 @@ typeofFunc(sql_context * context, int NotUsed, sql_value ** argv)
{
const char *z = 0;
UNUSED_PARAMETER(NotUsed);
+ enum field_type f_t = argv[0]->field_type;
+ /*
+ * SCALAR is not a basic type, but rather an aggregation of
+ * types. Thus, ignore SCALAR field type and return msgpack
+ * format type.
+ */
+ if (f_t != field_type_MAX && f_t != FIELD_TYPE_SCALAR) {
+ sql_result_text(context, field_type_strs[argv[0]->field_type],
+ -1, SQL_STATIC);
+ return;
+ }
switch (sql_value_type(argv[0])) {
case MP_INT:
case MP_UINT:
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 9142932e9..2c7a1c2da 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1238,6 +1238,7 @@ case OP_SoftNull: {
assert(pOp->p1>0 && pOp->p1<=(p->nMem+1 - p->nCursor));
pOut = &aMem[pOp->p1];
pOut->flags = (pOut->flags|MEM_Null)&~MEM_Undefined;
+ pOut->field_type = field_type_MAX;
break;
}
@@ -1278,6 +1279,11 @@ case OP_Variable: { /* out2 */
}
pOut = out2Prerelease(p, pOp);
sqlVdbeMemShallowCopy(pOut, pVar, MEM_Static);
+ /*
+ * ...ShallowCopy() may overwrite field_type with irrelevant
+ * value. So make sure that in the end field_type_MAX is set.
+ */
+ pOut->field_type = field_type_MAX;
UPDATE_MAX_BLOBSIZE(pOut);
break;
}
@@ -1338,6 +1344,7 @@ case OP_Copy: {
pIn1 = &aMem[pOp->p1];
pOut = &aMem[pOp->p2];
assert(pOut!=pIn1);
+
while( 1) {
sqlVdbeMemShallowCopy(pOut, pIn1, MEM_Ephem);
Deephemeralize(pOut);
@@ -1389,6 +1396,7 @@ case OP_IntCopy: { /* out2 */
assert((pIn1->flags & (MEM_Int | MEM_UInt)) != 0);
pOut = &aMem[pOp->p2];
mem_set_int(pOut, pIn1->u.i, (pIn1->flags & MEM_Int) != 0);
+ pOut->field_type = field_type_MAX;
break;
}
@@ -1491,6 +1499,8 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */
assert(pIn1!=pOut);
if ((pIn1->flags | pIn2->flags) & MEM_Null) {
sqlVdbeMemSetNull(pOut);
+ /* Force NULL be of type STRING. */
+ pOut->field_type = FIELD_TYPE_STRING;
break;
}
/*
@@ -1535,6 +1545,7 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */
pOut->z[nByte+1] = 0;
pOut->flags |= MEM_Term;
pOut->n = (int)nByte;
+ pOut->field_type = field_type_MAX;
UPDATE_MAX_BLOBSIZE(pOut);
break;
}
@@ -1644,6 +1655,7 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */
}
}
mem_set_int(pOut, iB, is_res_neg);
+ pOut->field_type = field_type_MAX;
} else {
bIntint = 0;
if (sqlVdbeRealValue(pIn1, &rA) != 0) {
@@ -1681,6 +1693,7 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */
}
pOut->u.r = rB;
MemSetTypeFlag(pOut, MEM_Real);
+ pOut->field_type = field_type_MAX;
if (((type1|type2)&MEM_Real)==0 && !bIntint) {
mem_apply_integer_type(pOut);
}
@@ -1689,6 +1702,8 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */
arithmetic_result_is_null:
sqlVdbeMemSetNull(pOut);
+ /* Force NULL be of type NUMBER. */
+ pOut->field_type = FIELD_TYPE_NUMBER;
break;
division_by_zero:
@@ -1909,6 +1924,7 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */
}
}
mem_set_i64(pOut, iA);
+ pOut->field_type = field_type_MAX;
break;
}
@@ -1988,6 +2004,13 @@ case OP_Cast: { /* in1 */
if (ExpandBlob(pIn1) != 0)
goto abort_due_to_error;
rc = sqlVdbeMemCast(pIn1, pOp->p2);
+ /*
+ * SCALAR is not type itself, but rather an aggregation
+ * of types. Hence, cast to this type shouldn't change
+ * original type of argument.
+ */
+ if (pOp->p2 != FIELD_TYPE_SCALAR)
+ pIn1->field_type = pOp->p2;
UPDATE_MAX_BLOBSIZE(pIn1);
if (rc == 0)
break;
@@ -2094,6 +2117,8 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
pIn3 = &aMem[pOp->p3];
flags1 = pIn1->flags;
flags3 = pIn3->flags;
+ enum field_type ft_p1 = pIn1->field_type;
+ enum field_type ft_p3 = pIn3->field_type;
if ((flags1 | flags3)&MEM_Null) {
/* One or both operands are NULL */
if (pOp->p5 & SQL_NULLEQ) {
@@ -2232,8 +2257,10 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
/* Undo any changes made by mem_apply_type() to the input registers. */
assert((pIn1->flags & MEM_Dyn) == (flags1 & MEM_Dyn));
pIn1->flags = flags1;
+ pIn1->field_type = ft_p1;
assert((pIn3->flags & MEM_Dyn) == (flags3 & MEM_Dyn));
pIn3->flags = flags3;
+ pIn3->field_type = ft_p3;
if (pOp->p5 & SQL_STOREP2) {
pOut = &aMem[pOp->p2];
@@ -2451,6 +2478,7 @@ case OP_Or: { /* same as TK_OR, in1, in2, out3 */
} else {
mem_set_bool(pOut, v1);
}
+ pOut->field_type = field_type_MAX;
break;
}
@@ -2472,6 +2500,7 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */
goto abort_due_to_error;
}
mem_set_bool(pOut, ! pIn1->u.b);
+ pOut->field_type = field_type_MAX;
}
break;
}
@@ -2645,7 +2674,7 @@ case OP_Column: {
}
pC->cacheStatus = p->cacheCtr;
}
- enum field_type field_type = FIELD_TYPE_ANY;
+ enum field_type field_type = field_type_MAX;
if (pC->eCurType == CURTYPE_TARANTOOL) {
/*
* Ephemeral spaces feature only one index
@@ -2660,6 +2689,8 @@ case OP_Column: {
field_type = pC->uc.pCursor->space->def->
fields[p2].type;
}
+ } else if (pC->eCurType == CURTYPE_SORTER) {
+ field_type = vdbe_sorter_get_field_type(pC->uc.pSorter, p2);
}
struct Mem *default_val_mem =
pOp->p4type == P4_MEM ? pOp->p4.pMem : NULL;
@@ -2679,6 +2710,7 @@ case OP_Column: {
sqlVdbeMemSetDouble(pDest, pDest->u.u);
}
}
+ pDest->field_type = field_type;
op_column_out:
REGISTER_TRACE(p, pOp->p3, pDest);
break;
@@ -3750,6 +3782,7 @@ case OP_FCopy: { /* out2 */
if ((pOp->p3 & OPFLAG_NOOP_IF_NULL) && (pIn1->flags & MEM_Null)) {
pOut = &aMem[pOp->p2];
if (pOut->flags & MEM_Undefined) pOut->flags = MEM_Null;
+ pOut->field_type = field_type_MAX;
/* Flag is set and register is NULL -> do nothing */
} else {
assert(memIsValid(pIn1));
@@ -3757,6 +3790,7 @@ case OP_FCopy: { /* out2 */
pOut = &aMem[pOp->p2];
mem_set_int(pOut, pIn1->u.i, pIn1->flags == MEM_Int);
+ pOut->field_type = pIn1->field_type;
}
break;
}
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index b83926a66..e98e01c8e 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -174,6 +174,13 @@ struct Mem {
u32 flags; /* Some combination of MEM_Null, MEM_Str, MEM_Dyn, etc. */
/** Subtype for this value. */
enum sql_subtype subtype;
+ /**
+ * If value is fetched from tuple, then this property
+ * contains type of corresponding space's field. If it's
+ * value field_type_MAX then we can rely on on format
+ * (msgpack) type which is represented by 'flags'.
+ */
+ enum field_type field_type;
int n; /* size (in bytes) of string value, excluding trailing '\0' */
char *z; /* String or BLOB value */
/* ShallowCopy only needs to copy the information above */
@@ -518,6 +525,10 @@ int sqlVdbeFrameRestore(VdbeFrame *);
int sqlVdbeSorterInit(struct sql *db, struct VdbeCursor *cursor);
void sqlVdbeSorterReset(sql *, VdbeSorter *);
+
+enum field_type
+vdbe_sorter_get_field_type(struct VdbeSorter *sorter, uint32_t field_no);
+
void sqlVdbeSorterClose(sql *, VdbeCursor *);
int sqlVdbeSorterRowkey(const VdbeCursor *, Mem *);
int sqlVdbeSorterNext(sql *, const VdbeCursor *, int *);
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index ecf1b3601..c62cb7a1e 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -620,6 +620,7 @@ columnNullValue(void)
0},
/* .flags = */ (u16) MEM_Null,
/* .eSubtype = */ (u8) 0,
+ /* .field_type = */ field_type_MAX,
/* .n = */ (int)0,
/* .z = */ (char *)0,
/* .zMalloc = */ (char *)0,
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index e153c150f..e9adf7bfa 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -1280,6 +1280,7 @@ initMemArray(Mem * p, int N, sql * db, u32 flags)
p->db = db;
p->flags = flags;
p->szMalloc = 0;
+ p->field_type = field_type_MAX;
#ifdef SQL_DEBUG
p->pScopyFrom = 0;
#endif
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 847a6b0ce..e5941de8e 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -327,6 +327,7 @@ sqlVdbeMemFinalize(Mem * pMem, FuncDef * pFunc)
memset(&t, 0, sizeof(t));
t.flags = MEM_Null;
t.db = pMem->db;
+ t.field_type = field_type_MAX;
ctx.pOut = &t;
ctx.pMem = pMem;
ctx.pFunc = pFunc;
@@ -747,6 +748,7 @@ sqlVdbeMemInit(Mem * pMem, sql * db, u32 flags)
pMem->flags = flags;
pMem->db = db;
pMem->szMalloc = 0;
+ pMem->field_type = field_type_MAX;
}
/*
@@ -892,6 +894,7 @@ sqlVdbeMemAboutToChange(Vdbe * pVdbe, Mem * pMem)
for (i = 0, pX = pVdbe->aMem; i < pVdbe->nMem; i++, pX++) {
if (pX->pScopyFrom == pMem) {
pX->flags |= MEM_Undefined;
+ pX->field_type = field_type_MAX;
pX->pScopyFrom = 0;
}
}
diff --git a/src/box/sql/vdbesort.c b/src/box/sql/vdbesort.c
index fc89ee721..a2d681255 100644
--- a/src/box/sql/vdbesort.c
+++ b/src/box/sql/vdbesort.c
@@ -968,6 +968,12 @@ sqlVdbeSorterReset(sql * db, VdbeSorter * pSorter)
pSorter->pUnpacked = 0;
}
+enum field_type
+vdbe_sorter_get_field_type(struct VdbeSorter *sorter, uint32_t field_no)
+{
+ return sorter->key_def->parts[field_no].type;
+}
+
/*
* Free any cursor components allocated by sqlVdbeSorterXXX routines.
*/
diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index 9fc4ff11f..f06cc81d6 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -140,7 +140,7 @@ test:do_execsql_test(
SELECT typeof(CAST(NULL AS text))
]], {
-- <cast-1.14>
- "null"
+ "string"
-- </cast-1.14>
})
@@ -160,7 +160,7 @@ test:do_execsql_test(
SELECT typeof(CAST(NULL AS FLOAT))
]], {
-- <cast-1.16>
- "null"
+ "number"
-- </cast-1.16>
})
@@ -200,7 +200,7 @@ test:do_execsql_test(
SELECT typeof(CAST(NULL AS integer))
]], {
-- <cast-1.20>
- "null"
+ "integer"
-- </cast-1.20>
})
@@ -511,7 +511,7 @@ test:do_execsql_test(
SELECT typeof(CAST(null AS REAL))
]], {
-- <case-1.61>
- "null"
+ "number"
-- </case-1.61>
})
diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
index a539f5222..7f4b15e72 100755
--- a/test/sql-tap/table.test.lua
+++ b/test/sql-tap/table.test.lua
@@ -906,7 +906,7 @@ test:do_execsql_test(
FROM t7 LIMIT 1;
]], {
-- <table-11.1>
- "integer", "null", "null", "null", "null", "null", "null", "null"
+ "integer", "number", "string", "string", "null", "null", "string", "string"
-- </table-11.1>
})
@@ -917,7 +917,7 @@ test:do_execsql_test(
FROM t7 LIMIT 1;
]], {
-- <table-11.2>
- "null", "null", "null", "null"
+ "number", "string", "number", "string"
-- </table-11.2>
})
diff --git a/test/sql/types.result b/test/sql/types.result
index 4589b2d58..6e7fffa42 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -636,7 +636,7 @@ box.execute("SELECT typeof(b) FROM t;")
rows:
- ['boolean']
- ['boolean']
- - ['null']
+ - ['boolean']
...
box.execute("SELECT quote(b) FROM t;")
---
@@ -1751,3 +1751,36 @@ box.execute("SELECT CAST('-123' AS UNSIGNED);")
box.space.T1:drop()
---
...
+-- gh-4148: make sure that typeof() returns origin type of column
+-- even if value is null.
+--
+box.execute("CREATE TABLE t (id INT PRIMARY KEY, a INT, s SCALAR);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t VALUES (1, 1, 1), (2, NULL, NULL);")
+---
+- row_count: 2
+...
+box.execute("SELECT typeof(a), typeof(s) FROM t;")
+---
+- metadata:
+ - name: typeof(a)
+ type: string
+ - name: typeof(s)
+ type: string
+ rows:
+ - ['integer', 'integer']
+ - ['integer', 'null']
+...
+box.execute("SELECT typeof(CAST(0 AS UNSIGNED));")
+---
+- metadata:
+ - name: typeof(CAST(0 AS UNSIGNED))
+ type: string
+ rows:
+ - ['unsigned']
+...
+box.space.T:drop()
+---
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index f07a90b37..489a5a91c 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -399,3 +399,14 @@ box.execute("SELECT CAST('123' AS UNSIGNED);")
box.execute("SELECT CAST('-123' AS UNSIGNED);")
box.space.T1:drop()
+
+-- gh-4148: make sure that typeof() returns origin type of column
+-- even if value is null.
+--
+box.execute("CREATE TABLE t (id INT PRIMARY KEY, a INT, s SCALAR);")
+box.execute("INSERT INTO t VALUES (1, 1, 1), (2, NULL, NULL);")
+box.execute("SELECT typeof(a), typeof(s) FROM t;")
+
+box.execute("SELECT typeof(CAST(0 AS UNSIGNED));")
+
+box.space.T:drop()
--
2.15.1
More information about the Tarantool-patches
mailing list