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 7CCB12A40E for ; Mon, 1 Apr 2019 16:42:47 -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 rklYH5oI5-Z9 for ; Mon, 1 Apr 2019 16:42:47 -0400 (EDT) Received: from smtp62.i.mail.ru (smtp62.i.mail.ru [217.69.128.42]) (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 2E7F32A401 for ; Mon, 1 Apr 2019 16:42:47 -0400 (EDT) From: Stanislav Zudin Subject: [tarantool-patches] Re: [PATCH 02/13] sql: make VDBE recognize big integers References: <250778a6f016d3be9eb288255cea0e4a7c7b22c5.1552663061.git.szudin@tarantool.org> <45D42A54-BB4A-4F52-AA10-F13B626AD07E@tarantool.org> Message-ID: <65816c3f-531d-dd3d-cbb4-33ec4ae560aa@tarantool.org> Date: Mon, 1 Apr 2019 23:42:44 +0300 MIME-Version: 1.0 In-Reply-To: <45D42A54-BB4A-4F52-AA10-F13B626AD07E@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit 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, "n.pettik" On 25.03.2019 18:11, n.pettik wrote: > > >> On 15 Mar 2019, at 18:45, Stanislav Zudin wrote: >> >> VDBE distinguishes signed and unsigned integers. >> SELECT query correctly returns values greater >> than INT64_MAX. > > Please, add tests on these changes. > >> diff --git a/src/box/lua/sql.c b/src/box/lua/sql.c >> index ee20faab7..cb2927144 100644 >> --- a/src/box/lua/sql.c >> +++ b/src/box/lua/sql.c >> @@ -32,7 +32,10 @@ lua_push_row(struct lua_State *L, struct sql_stmt *stmt) >> int type = sql_column_type(stmt, i); >> switch (type) { >> case SQL_INTEGER: >> - luaL_pushint64(L, sql_column_int64(stmt, i)); >> + if (sql_column_is_unsigned(stmt, i)) >> + luaL_pushuint64(L, sql_column_int64(stmt, i)); >> + else >> + luaL_pushint64(L, sql_column_int64(stmt, i)); >> break; >> case SQL_FLOAT: >> lua_pushnumber(L, sql_column_double(stmt, i)); >> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h >> index 0ef373c24..92e2f282f 100644 >> --- a/src/box/sql/sqlInt.h >> +++ b/src/box/sql/sqlInt.h >> @@ -586,6 +586,9 @@ sql_column_text(sql_stmt *, >> int >> sql_column_type(sql_stmt *, int iCol); >> >> +bool >> +sql_column_is_unsigned(sql_stmt *, int iCol); >> + >> sql_value * >> sql_column_value(sql_stmt *, >> int iCol); >> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c >> index c87b10757..ea398e7d6 100644 >> --- a/src/box/sql/vdbe.c >> +++ b/src/box/sql/vdbe.c >> @@ -1141,6 +1141,8 @@ case OP_Int64: { /* out2 */ >> pOut = out2Prerelease(p, pOp); >> assert(pOp->p4.pI64!=0); >> pOut->u.i = *pOp->p4.pI64; >> + if (pOp->p4type == P4_UINT64) >> + pOut->flags = MEM_Int | MEM_Unsigned; > > Why combination of flags? Doesn’t Unsigned implie that it is integer? It caused less changes in VDBE. The 2-nd version of the patch handles 'unsigned' as independent type. > >> diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h >> index f9bb96f09..d4383901c 100644 >> --- a/src/box/sql/vdbe.h >> +++ b/src/box/sql/vdbe.h >> @@ -70,7 +70,7 @@ struct VdbeOp { >> int i; /* Integer value if p4type==P4_INT32 */ >> void *p; /* Generic pointer */ >> char *z; /* Pointer to data for string (char array) types */ >> - i64 *pI64; /* Used when p4type is P4_INT64 */ >> + i64 *pI64; /* Used when p4type is P4_INT64 or P4_UINT64 */ > > Please, add a separate member to union. > >> diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h >> index 61b7d58b2..66b21299a 100644 >> --- a/src/box/sql/vdbeInt.h >> +++ b/src/box/sql/vdbeInt.h >> @@ -248,6 +248,9 @@ struct Mem { >> #define MEM_Agg 0x4000 /* Mem.z points to an agg function context */ >> #define MEM_Zero 0x8000 /* Mem.i contains count of 0s appended to blob */ >> #define MEM_Subtype 0x10000 /* Mem.eSubtype is valid */ >> +#define MEM_Unsigned 0x20000 /* Value is unsigned integer. >> + * Combine this flag with MEM_Int >> + * if necessary */ >> #ifdef SQL_OMIT_INCRBLOB >> #undef MEM_Zero >> #define MEM_Zero 0x0000 > > Garbage I guess. Otherwise, please describe this change. ??? Which one? > >> diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c >> index d7e89073e..02a1b2e0f 100644 >> --- a/src/box/sql/vdbeapi.c >> +++ b/src/box/sql/vdbeapi.c >> @@ -1044,6 +1044,14 @@ sql_column_type(sql_stmt * pStmt, int i) >> return iType; >> } >> >> +bool sql_column_is_unsigned(sql_stmt * pStmt, int i) > > Bad formatting. Should be: > > bool > sql_column_is_unsigned... This function was removed. > >> +{ >> + const struct Mem* pMem = columnMem(pStmt, i); >> + if (!pMem) >> + return false; > > Is this check necessary? Other column-like function from this > set don’t provide it. > >> + return (pMem->flags & MEM_Unsigned); >> +} >> + >> enum sql_subtype >> sql_column_subtype(struct sql_stmt *stmt, int i) >> { > >