[tarantool-patches] Re: [PATCH 04/13] sql: support big integers within sql binding
Stanislav Zudin
szudin at tarantool.org
Mon Apr 1 23:42:59 MSK 2019
On 25.03.2019 18:12, n.pettik wrote:
>
>> 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 = mp_decode_uint(packet);
>> - if (n > INT64_MAX) {
>> - diag_set(ClientError, ER_SQL_BIND_VALUE,
>> - sql_bind_name(bind), "INTEGER");
>> - return -1;
>> - }
>> bind->i64 = (int64_t) n;
>
> Please, add to bind struct separate member to hold uints.
Done.
>
>> - bind->type = SQL_INTEGER;
>> + bind->type = (n > INT64_MAX) ? SQL_UNSIGNED : SQL_INTEGER;
>> bind->bytes = sizeof(bind->i64);
>> break;
>> }
>> @@ -246,6 +241,7 @@ sql_column_to_messagepack(struct sql_stmt *stmt, int i,
>> int type = sql_column_type(stmt, i);
>> switch (type) {
>> case SQL_INTEGER: {
>> + assert(!sql_column_is_unsigned(stmt, i));
>> int64_t n = sql_column_int64(stmt, i);
>> if (n >= 0)
>> size = 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 = sql_column_int64(stmt, i);
>
> ->sql_column_uint64()
>
Done.
>> + size = mp_sizeof_uint(n);
>> + char *pos = (char *) region_alloc(region, size);
>> + if (pos == NULL)
>> + goto oom;
>> + mp_encode_uint(pos, n);
>> + break;
>> + }
>>
>> 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’s introduce sql_value_uint64().
Done.
>
>> 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 = 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
>>
>> @@ -191,6 +192,11 @@ absFunc(sql_context * context, int argc, sql_value ** argv)
>> sql_result_int64(context, iVal);
>> break;
>> }
>> + case SQL_UNSIGNED: {
>> + i64 iVal = 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
>> /**
>>
>> 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 = &aMem[pOp->p1];
>> assert((pIn1->flags & MEM_Int)!=0);
>> pOut = &aMem[pOp->p2];
>> - sqlVdbeMemSetInt64(pOut, pIn1->u.i);
>> + sqlVdbeMemSetInt64(pOut, pIn1->u.i, (pIn1->flags & MEM_Unsigned)!=0);
>> break;
>> }
>>
>> @@ -1770,7 +1770,7 @@ integer_overflow:
>> case OP_CollSeq: {
>> assert(pOp->p4type==P4_COLLSEQ || pOp->p4.pColl == 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.
Done.
>
>> }
>> break;
>> }
>> @@ -5317,7 +5317,7 @@ case OP_AggStep: {
>> if (pCtx->skipFlag) {
>> assert(pOp[-1].opcode==OP_CollSeq);
>> i = 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.
>
Done.
>> };
>> - return aType[pVal->flags & MEM_PURE_TYPE_MASK];
>> +
>> + assert(MEM_Unsigned == 0x20000);
>> + /* compress the unsigned bit with the pure
>> + * type bits, to make them applicable for
>> + * array indexing.
>> + */
>> + u32 offset = (pVal->flags >> 12) | (pVal->flags & MEM_PURE_TYPE_MASK);
>> + assert(offset < 0x40);
>> + return aType[offset];
>> }
>>
>> /* Make a copy of an sql_value object
>> @@ -401,13 +442,13 @@ sql_result_error(sql_context * pCtx, const char *z, int n)
>>
>> void
>> @@ -1370,7 +1411,20 @@ sql_bind_int64(sql_stmt * pStmt, int i, sql_int64 iValue)
>> rc = vdbeUnbind(p, i);
>> if (rc == SQL_OK) {
>> rc = 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 = (Vdbe *) pStmt;
>> + rc = vdbeUnbind(p, i);
>> + if (rc == SQL_OK) {
>> + rc = sql_bind_type(p, i, "INTEGER”);
>
> Why integer? I guess it should be unsigned.
Fixed.
>
>> + sqlVdbeMemSetInt64(&p->aVar[i - 1], (u64)iValue, true);
>> }
>> return rc;
>> }
>>
>
>
More information about the Tarantool-patches
mailing list