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 3F56729C02 for ; Mon, 25 Mar 2019 11:12:36 -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 NbXvXaM3xMqY for ; Mon, 25 Mar 2019 11:12:36 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 CA93829B67 for ; Mon, 25 Mar 2019 11:12:35 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH 04/13] sql: support big integers within sql binding From: "n.pettik" In-Reply-To: Date: Mon, 25 Mar 2019 18:12:33 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <7B68C4B8-A53B-483E-AD57-8A746682FD73@tarantool.org> 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: szudin@tarantool.org > diff --git a/src/box/execute.c b/src/box/execute.c > index 7c77df2e5..31b89a75e 100644 > --- a/src/box/execute.c > +++ b/src/box/execute.c > @@ -130,13 +130,8 @@ sql_bind_decode(struct sql_bind *bind, int i, = const char **packet) > switch (mp_typeof(**packet)) { > case MP_UINT: { > uint64_t n =3D mp_decode_uint(packet); > - if (n > INT64_MAX) { > - diag_set(ClientError, ER_SQL_BIND_VALUE, > - sql_bind_name(bind), "INTEGER"); > - return -1; > - } > bind->i64 =3D (int64_t) n; Please, add to bind struct separate member to hold uints. > - bind->type =3D SQL_INTEGER; > + bind->type =3D (n > INT64_MAX) ? SQL_UNSIGNED : = SQL_INTEGER; > bind->bytes =3D sizeof(bind->i64); > break; > } > @@ -246,6 +241,7 @@ sql_column_to_messagepack(struct sql_stmt *stmt, = int i, > int type =3D sql_column_type(stmt, i); > switch (type) { > case SQL_INTEGER: { > + assert(!sql_column_is_unsigned(stmt, i)); > int64_t n =3D sql_column_int64(stmt, i); > if (n >=3D 0) > size =3D mp_sizeof_uint(n); > @@ -260,6 +256,16 @@ sql_column_to_messagepack(struct sql_stmt *stmt, = int i, > mp_encode_int(pos, n); > break; > } > + case SQL_UNSIGNED: { > + assert(sql_column_is_unsigned(stmt, i)); > + int64_t n =3D sql_column_int64(stmt, i); ->sql_column_uint64() > + size =3D mp_sizeof_uint(n); > + char *pos =3D (char *) region_alloc(region, size); > + if (pos =3D=3D NULL) > + goto oom; > + mp_encode_uint(pos, n); > + break; > + } >=20 > diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c > index f5a7b7819..57a3161c7 100644 > --- a/src/box/lua/lua_sql.c > +++ b/src/box/lua/lua_sql.c > @@ -60,6 +60,9 @@ lua_sql_call(sql_context *pCtx, int nVal, sql_value = **apVal) { > case SQL_INTEGER: > luaL_pushint64(L, sql_value_int64(param)); > break; > + case SQL_UNSIGNED: > + luaL_pushuint64(L, sql_value_int64(param)); Let=E2=80=99s introduce sql_value_uint64(). > diff --git a/src/box/lua/sql.c b/src/box/lua/sql.c > index cb2927144..9a35c03aa 100644 > --- a/src/box/lua/sql.c > +++ b/src/box/lua/sql.c > @@ -32,10 +32,12 @@ lua_push_row(struct lua_State *L, struct sql_stmt = *stmt) > int type =3D sql_column_type(stmt, i); > switch (type) { > case SQL_INTEGER: > - if (sql_column_is_unsigned(stmt, i)) > - luaL_pushuint64(L, = sql_column_int64(stmt, i)); > - else > - luaL_pushint64(L, sql_column_int64(stmt, = i)); > + assert(!sql_column_is_unsigned(stmt, i)); > + luaL_pushint64(L, sql_column_int64(stmt, i)); > + break; > + case SQL_UNSIGNED: > + assert(sql_column_is_unsigned(stmt, i)); > + luaL_pushuint64(L, sql_column_int64(stmt, i)); > break; > case SQL_FLOAT: > lua_pushnumber(L, sql_column_double(stmt, i)); > diff --git a/src/box/sql/func.c b/src/box/sql/func.c > index 21a69aa51..cf65bf2a2 100644 > --- a/src/box/sql/func.c > +++ b/src/box/sql/func.c >=20 > @@ -191,6 +192,11 @@ absFunc(sql_context * context, int argc, = sql_value ** argv) > sql_result_int64(context, iVal); > break; > } > + case SQL_UNSIGNED: { > + i64 iVal =3D sql_value_int64(argv[0]); Replace this with uints once you add sql_value_uint64(). > + sql_result_int64(context, iVal); The same: lets add sql_result_uint64(). > + break; > + } > case SQL_NULL:{ > /* IMP: R-37434-19929 Abs(X) returns NULL if X = is NULL. */ > sql_result_null(context); > diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h > index 92e2f282f..56aa7c681 100644 > --- a/src/box/sql/sqlInt.h > +++ b/src/box/sql/sqlInt.h > /** >=20 > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index ea398e7d6..8a7f7a12f 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -1439,7 +1439,7 @@ case OP_IntCopy: { /* out2 */ > pIn1 =3D &aMem[pOp->p1]; > assert((pIn1->flags & MEM_Int)!=3D0); > pOut =3D &aMem[pOp->p2]; > - sqlVdbeMemSetInt64(pOut, pIn1->u.i); > + sqlVdbeMemSetInt64(pOut, pIn1->u.i, (pIn1->flags & = MEM_Unsigned)!=3D0); > break; > } >=20 > @@ -1770,7 +1770,7 @@ integer_overflow: > case OP_CollSeq: { > assert(pOp->p4type=3D=3DP4_COLLSEQ || pOp->p4.pColl =3D=3D = NULL); > if (pOp->p1) { > - sqlVdbeMemSetInt64(&aMem[pOp->p1], 0); > + sqlVdbeMemSetInt64(&aMem[pOp->p1], 0, false); Instead of passing argument indicating signedness, I would introduce separate function to set vdbe memory with uint value.=20 > } > break; > } > @@ -5317,7 +5317,7 @@ case OP_AggStep: { > if (pCtx->skipFlag) { > assert(pOp[-1].opcode=3D=3DOP_CollSeq); > i =3D pOp[-1].p1; > - if (i) sqlVdbeMemSetInt64(&aMem[i], 1); > + if (i) sqlVdbeMemSetInt64(&aMem[i], 1, false); > } > break; > } > diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h > index 66b21299a..0375845d9 100644 > --- a/src/box/sql/vdbeInt.h > +++ b/src/box/sql/vdbeInt.h > @@ -477,7 +477,7 @@ void sqlVdbeMemShallowCopy(Mem *, const Mem *, = int); > void sqlVdbeMemMove(Mem *, Mem *); > int sqlVdbeMemNulTerminate(Mem *); > int sqlVdbeMemSetStr(Mem *, const char *, int, u8, void (*)(void *)); > -void sqlVdbeMemSetInt64(Mem *, i64); > +void sqlVdbeMemSetInt64(Mem *, i64, bool); > #ifdef SQL_OMIT_FLOATING_POINT > #define sqlVdbeMemSetDouble sqlVdbeMemSetInt64 > #else > diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c > index 02a1b2e0f..2c486552e 100644 > --- a/src/box/sql/vdbeapi.c > +++ b/src/box/sql/vdbeapi.c > @@ -279,8 +279,49 @@ sql_value_type(sql_value * pVal) > SQL_NULL, /* 0x1d */ > SQL_INTEGER, /* 0x1e */ > SQL_NULL, /* 0x1f */ > + > + SQL_BLOB, /* 0x20 */ > + SQL_NULL, /* 0x21 */ > + SQL_TEXT, /* 0x22 */ > + SQL_NULL, /* 0x23 */ > + SQL_UNSIGNED, /* 0x24 */ > + SQL_NULL, /* 0x25 */ > + SQL_UNSIGNED, /* 0x26 */ > + SQL_NULL, /* 0x27 */ > + SQL_FLOAT, /* 0x28 */ > + SQL_NULL, /* 0x29 */ > + SQL_FLOAT, /* 0x2a */ > + SQL_NULL, /* 0x2b */ > + SQL_INTEGER, /* 0x2c */ > + SQL_NULL, /* 0x2d */ > + SQL_INTEGER, /* 0x2e */ > + SQL_NULL, /* 0x2f */ > + SQL_BLOB, /* 0x30 */ > + SQL_NULL, /* 0x31 */ > + SQL_TEXT, /* 0x32 */ > + SQL_NULL, /* 0x33 */ > + SQL_INTEGER, /* 0x34 */ > + SQL_NULL, /* 0x35 */ > + SQL_INTEGER, /* 0x36 */ > + SQL_NULL, /* 0x37 */ > + SQL_FLOAT, /* 0x38 */ > + SQL_NULL, /* 0x39 */ > + SQL_FLOAT, /* 0x3a */ > + SQL_NULL, /* 0x3b */ > + SQL_INTEGER, /* 0x3c */ > + SQL_NULL, /* 0x3d */ > + SQL_INTEGER, /* 0x3e */ > + SQL_NULL, /* 0x3f */ Looks terrible. Could we rework this mechanism of fetching type? Soon we are going to add several types more, so the size of array will become enormous. > }; > - return aType[pVal->flags & MEM_PURE_TYPE_MASK]; > + > + assert(MEM_Unsigned =3D=3D 0x20000); > + /* compress the unsigned bit with the pure > + * type bits, to make them applicable for > + * array indexing. > + */ > + u32 offset =3D (pVal->flags >> 12) | (pVal->flags & = MEM_PURE_TYPE_MASK); > + assert(offset < 0x40); > + return aType[offset]; > } >=20 > /* Make a copy of an sql_value object > @@ -401,13 +442,13 @@ sql_result_error(sql_context * pCtx, const char = *z, int n) >=20 > void > @@ -1370,7 +1411,20 @@ sql_bind_int64(sql_stmt * pStmt, int i, = sql_int64 iValue) > rc =3D vdbeUnbind(p, i); > if (rc =3D=3D SQL_OK) { > rc =3D sql_bind_type(p, i, "INTEGER"); > - sqlVdbeMemSetInt64(&p->aVar[i - 1], iValue); > + sqlVdbeMemSetInt64(&p->aVar[i - 1], iValue, false); > + } > + return rc; > +} > + > +int > +sql_bind_uint64(sql_stmt * pStmt, int i, sql_uint64 iValue) > +{ > + int rc; > + Vdbe *p =3D (Vdbe *) pStmt; > + rc =3D vdbeUnbind(p, i); > + if (rc =3D=3D SQL_OK) { > + rc =3D sql_bind_type(p, i, "INTEGER=E2=80=9D); Why integer? I guess it should be unsigned. > + sqlVdbeMemSetInt64(&p->aVar[i - 1], (u64)iValue, true); > } > return rc; > } >=20