From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 1B2E52C7FC for ; Fri, 5 Oct 2018 12:41:25 -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 9yarbnAY9Uzi for ; Fri, 5 Oct 2018 12:41:25 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 BB2962C7F5 for ; Fri, 5 Oct 2018 12:41:23 -0400 (EDT) Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1g8TAH-0001oS-OQ for tarantool-patches@freelists.org; Fri, 05 Oct 2018 19:41:22 +0300 Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: assertion in autoincrement column References: <20180929042720.GC32712@chai> From: Imeev Mergen Message-ID: <924fdf95-65c1-46a4-f8a7-d397c81541dc@tarantool.org> Date: Fri, 5 Oct 2018 19:41:21 +0300 MIME-Version: 1.0 In-Reply-To: <20180929042720.GC32712@chai> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Hello! Thank you for review! New patch below. On 09/29/2018 07:27 AM, Konstantin Osipov wrote: > * imeevma@tarantool.org [18/09/28 19:18]: >> pOut = &aMem[pOp->p2]; >> - MemSetTypeFlag(pOut, MEM_Int); >> + /* >> + * Flag should be MEM_Int but in some cases it can >> + * have an other value. See gh-3670. >> + */ >> + MemSetTypeFlag(pOut, pIn1->flags); > Please make the comment more explicit. I should not have to dig > into commit history and find the relevant test case to understand > what's going on here. Done. > >> >> pOut->u.i = pIn1->u.i; >> } New patch: commit 53a229c67b096a09f26df4b5ba73d9a90c46e38f Author: Mergen Imeev Date:   Thu Sep 27 22:42:29 2018 +0300     sql: assertion in autoincrement column     If query is like "INSERT INTO table SELECT ..." then it is     completely fine that the result of SELECT isn't integer. This     value wasn't cheched propertly if it was inserted in column     with autoincrement.     Closes #3670 diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 7c1015c..de18262 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3736,10 +3736,15 @@ case OP_FCopy: {     /* out2 */          /* Flag is set and register is NULL -> do nothing  */      } else {          assert(memIsValid(pIn1)); -        assert(pIn1->flags &  MEM_Int);          pOut = &aMem[pOp->p2]; -        MemSetTypeFlag(pOut, MEM_Int); +        /* +         * If query is like "INSERT INTO table SELECT ..." +         * then it is possible then the result of SELECT +         * isn't MEM_Int. It will be checked later in +         * current VDBE. +         */ +        MemSetTypeFlag(pOut, pIn1->flags);          pOut->u.i = pIn1->u.i;      } diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua index dda7061..bd40de9 100755 --- a/test/sql-tap/autoinc.test.lua +++ b/test/sql-tap/autoinc.test.lua @@ -1,6 +1,6 @@  #!/usr/bin/env tarantool  test = require("sqltester") -test:plan(46) +test:plan(48)  --!./tcltestrunner.lua  -- 2004 November 12 @@ -801,4 +801,29 @@ test:do_test(          --      }) +-- gh-3670: Assertion with large number in autoincrement column +test:do_catchsql_test( +    "autoinc-gh-3670-1", +    [[ +        CREATE TABLE t0 (s1 INT PRIMARY KEY AUTOINCREMENT); +        INSERT INTO t0 VALUES (2147483647); +        INSERT INTO t0 SELECT max(s1)*max(s1)*max(s1) FROM t0; +    ]], { +        -- +        1, "datatype mismatch" +        -- +    }) + +test:do_catchsql_test( +    "autoinc-gh-3670-2", +    [[ +        CREATE TABLE t1 (s1 INT PRIMARY KEY AUTOINCREMENT, s2 CHAR); +        INSERT INTO t1 VALUES (1, 'a'); +        INSERT INTO t1 SELECT s2, s2 FROM t1; +    ]], { +        -- +        1, "datatype mismatch" +        -- +    }) +  test:finish_test()