[tarantool-patches] Re: [PATCH 6/6] sql: allow to specify UNSIGNED column type
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Tue Jul 2 00:53:02 MSK 2019
There still are places, which access Mem.u.i/u without
checking flags properly. Please, for each place either
explain why it is correct, or fix it and add a test.
--------------------------------------------------------------
vdbe.c:2659:
if ((pDest->flags & (MEM_Int | MEM_UInt)) != 0) {
if (field_type == FIELD_TYPE_NUMBER)
sqlVdbeMemSetDouble(pDest, pDest->u.i);
}
You can't access pDest->u.i - it can be a big unsigned.
--------------------------------------------------------------
vdbe.c:2955:
pIn1 = &aMem[pOp->p1];
uint32_t space_id = pIn1->u.i;
Here you touch u.i, but it is always
unsigned. You should use u.u, I think.
--------------------------------------------------------------
vdbeaux.c:2428:
if (flags & MEM_Int) {
...
i64 i = pMem->u.i;
u64 u;
if (i < 0) {
u = ~i;
} else {
u = i;
}
Why do you check 'i < 0', if you already
see its flag is 'Int'. It should be < 0 by
definition.
--------------------------------------------------------------
vdbeaux.c:2589:
u64 v;
u32 i;
if (serial_type == 7) {
assert(sizeof(v) == sizeof(pMem->u.r));
memcpy(&v, &pMem->u.r, sizeof(v));
swapMixedEndianFloat(v);
} else {
v = pMem->u.i;
}
Why are you sure it is safe to store u.i into
uint64_t variable?
--------------------------------------------------------------
vdbeaux.c:2644:
u64 x = FOUR_BYTE_UINT(buf);
u32 y = FOUR_BYTE_UINT(buf + 4);
x = (x << 32) + y;
if (serial_type == 6) {
/* EVIDENCE-OF: R-29851-52272 Value is a big-endian 64-bit
* twos-complement integer.
*/
pMem->u.i = *(i64 *) & x;
pMem->flags = MEM_Int;
Vice versa - why is it safe to store uint64_t into u.i,
and set its flag to 'Int' (== value is negative).
Actually, all this 'serial' shit looks broken. Please,
verify that code.
--------------------------------------------------------------
vdbeaux.c:2998:
if ((f1 & MEM_UInt) != 0) {
if ((f2 & MEM_Real) != 0) {
return sqlIntFloatCompare(pMem1->u.i,
pMem1 is unsigned, according to the first check,
but you use u.i. Why?
--------------------------------------------------------------
vdbeaux.c:3184:
do_int:
if ((pKey2->flags & (MEM_Int | MEM_UInt)) != 0) {
if (mem1.u.i < pKey2->u.i) {
rc = -1;
pKey2 is an integer, but you don't know the sign. And use u.i,
assuming it is negative, or at least small enough. Why?
Line 3207 is the same.
--------------------------------------------------------------
vdbemem.c:1431:
} else if (pVal->u.i == SMALLEST_INT64) {
pVal->u.r = -(double)SMALLEST_INT64;
MemSetTypeFlag(pVal, MEM_Real);
} else {
pVal->u.i = -pVal->u.i;
}
You compare u.i and SMALLEST_INT64, but you can't
be sure, that u.i is not a big unsigned, can you?
--------------------------------------------------------------
I will verify u.u, MEM_Int, and MEM_UInt usages, when we
finish with u.i.
More information about the Tarantool-patches
mailing list