[tarantool-patches] Re: [PATCH 2/6] sql: separate VDBE memory holding positive and negative ints

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the fixes!

Consider new ones below, and on the branch
in a separate commit.

=====================================================

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 83c0e90f1..2be42faf2 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -260,7 +260,7 @@ allocateCursor(
 int
 mem_apply_numeric_type(struct Mem *record)
 {
-	if ((record->flags & (MEM_Str | MEM_Int | MEM_Real | MEM_UInt)) != MEM_Str)
+	if ((record->flags & MEM_Str) == 0)
 		return -1;
 	int64_t integer_value;
 	bool is_neg;
@@ -936,7 +936,7 @@ case OP_Return: {           /* in1 */
 case OP_InitCoroutine: {     /* jump */
 	assert(pOp->p1>0 &&  pOp->p1<=(p->nMem+1 - p->nCursor));
 	assert(pOp->p2>=0 && pOp->p2<p->nOp);
-	assert(pOp->p3>=0 && pOp->p3<p->nOp);
+	assert(pOp->p3>0 && pOp->p3<p->nOp);
 	pOut = &aMem[pOp->p1];
 	assert(!VdbeMemDynamic(pOut));
 	mem_set_u64(pOut, pOp->p3 - 1);
@@ -1061,10 +1061,12 @@ case OP_Halt: {
  */
 case OP_Integer: {         /* out2 */
 	pOut = out2Prerelease(p, pOp);
-	if (pOp->p1 < 0)
+	if (pOp->p1 < 0) {
 		pOut->u.i = pOp->p1;
-	else
+		assert((pOut->flags & MEM_Int) != 0);
+	} else {
 		mem_set_u64(pOut, pOp->p1);
+	}
 	break;
 }
 
@@ -3307,8 +3309,7 @@ case OP_SeekGT: {       /* jump, in3 */
 		 * the seek, so convert it.
 		 */
 		pIn3 = &aMem[reg_ipk];
-		if ((pIn3->flags & (MEM_Int | MEM_UInt | MEM_Real |
-				    MEM_Str)) == MEM_Str)
+		if ((pIn3->flags & MEM_Str) != 0)
 			mem_apply_numeric_type(pIn3);
 		int64_t i;
 		if ((pIn3->flags & MEM_Int) == MEM_Int) {
@@ -4921,9 +4922,8 @@ case OP_OffsetLimit: {    /* in1, out2, in3 */
 case OP_IfNotZero: {        /* jump, in1 */
 	pIn1 = &aMem[pOp->p1];
 	assert((pIn1->flags & MEM_UInt) != 0);
-	VdbeBranchTaken(pIn1->u.u<0, 2);
-	if (pIn1->u.u) {
-		if (pIn1->u.u > 0) pIn1->u.u--;
+	if (pIn1->u.u > 0) {
+		pIn1->u.u--;
 		goto jump_to_p2;
 	}
 	break;
@@ -4938,8 +4938,8 @@ case OP_IfNotZero: {        /* jump, in1 */
 case OP_DecrJumpZero: {      /* jump, in1 */
 	pIn1 = &aMem[pOp->p1];
 	assert((pIn1->flags & MEM_UInt) != 0);
-	if (pIn1->u.i>SMALLEST_INT64) pIn1->u.u--;
-	VdbeBranchTaken(pIn1->u.i==0, 2);
+	if (pIn1->u.u > 0)
+		pIn1->u.u--;
 	if (pIn1->u.u == 0) goto jump_to_p2;
 	break;
 }