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 27FEB25597 for ; Sat, 27 Jul 2019 14:45:41 -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 M5k9-MqVT1gM for ; Sat, 27 Jul 2019 14:45:41 -0400 (EDT) Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107]) (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 6A860254D8 for ; Sat, 27 Jul 2019 14:45:40 -0400 (EDT) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 1/2] sql: extend struct Mem with field_type member Date: Sat, 27 Jul 2019 21:45:35 +0300 Message-Id: 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: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Nikita Pettik 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)) ]], { -- - "null" + "string" -- }) @@ -160,7 +160,7 @@ test:do_execsql_test( SELECT typeof(CAST(NULL AS FLOAT)) ]], { -- - "null" + "number" -- }) @@ -200,7 +200,7 @@ test:do_execsql_test( SELECT typeof(CAST(NULL AS integer)) ]], { -- - "null" + "integer" -- }) @@ -511,7 +511,7 @@ test:do_execsql_test( SELECT typeof(CAST(null AS REAL)) ]], { -- - "null" + "number" -- }) 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; ]], { -- - "integer", "null", "null", "null", "null", "null", "null", "null" + "integer", "number", "string", "string", "null", "null", "string", "string" -- }) @@ -917,7 +917,7 @@ test:do_execsql_test( FROM t7 LIMIT 1; ]], { -- - "null", "null", "null", "null" + "number", "string", "number", "string" -- }) 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