From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tarantool-patches-bounce@freelists.org>
Received: from localhost (localhost [127.0.0.1])
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 2BD7822B80
	for <tarantool-patches@freelists.org>; Mon,  1 Jul 2019 17:52:09 -0400 (EDT)
Received: from turing.freelists.org ([127.0.0.1])
	by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id 7QpMq_g_tdCL for <tarantool-patches@freelists.org>;
	Mon,  1 Jul 2019 17:52:09 -0400 (EDT)
Received: from smtp17.mail.ru (smtp17.mail.ru [94.100.176.154])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id D992F22BCF
	for <tarantool-patches@freelists.org>; Mon,  1 Jul 2019 17:52:08 -0400 (EDT)
Subject: [tarantool-patches] Re: [PATCH 2/6] sql: separate VDBE memory
 holding positive and negative ints
References: <cover.1559919361.git.korablev@tarantool.org>
 <0e009036b6c6ce9e2d1f2ff66063291cb60e1387.1559919361.git.korablev@tarantool.org>
 <6f7ec2c9-c325-601a-234f-97d6c6ced195@tarantool.org>
 <FF6FA731-9F47-4223-A264-66690C8ED766@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <09e60c9d-8a1e-38d8-a474-cfcbc586f9f4@tarantool.org>
Date: Mon, 1 Jul 2019 23:53:07 +0200
MIME-Version: 1.0
In-Reply-To: <FF6FA731-9F47-4223-A264-66690C8ED766@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: tarantool-patches-bounce@freelists.org
Errors-to: tarantool-patches-bounce@freelists.org
Reply-To: tarantool-patches@freelists.org
List-Help: <mailto:ecartis@freelists.org?Subject=help>
List-Unsubscribe: <mailto:tarantool-patches-request@freelists.org?Subject=unsubscribe>
List-software: Ecartis version 1.0.0
List-Id: tarantool-patches <tarantool-patches.freelists.org>
List-Subscribe: <mailto:tarantool-patches-request@freelists.org?Subject=subscribe>
List-Owner: <mailto:>
List-post: <mailto:tarantool-patches@freelists.org>
List-Archive: <http://www.freelists.org/archives/tarantool-patches>
To: "n.pettik" <korablev@tarantool.org>, tarantool-patches@freelists.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;
 }