[tarantool-patches] Re: [PATCH 02/13] sql: make VDBE recognize big integers

[n.pettik]

> On 15 Mar 2019, at 18:45, Stanislav Zudin <szudin at tarantool.org> 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?

> 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.

> 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...

> +{
> +	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)
> {