[tarantool-patches] Re: [PATCH 04/13] sql: support big integers within sql binding
n.pettik
korablev at tarantool.org
Mon Mar 25 18:12:33 MSK 2019
> 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.
> - 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()
> + 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().
> 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.
> }
> 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.
> };
> - 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.
> + sqlVdbeMemSetInt64(&p->aVar[i - 1], (u64)iValue, true);
> }
> return rc;
> }
>
More information about the Tarantool-patches
mailing list