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 B75712C38C for ; Thu, 11 Oct 2018 12:22:06 -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 IgmswN-q-SN8 for ; Thu, 11 Oct 2018 12:22:06 -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 40D442C1BB for ; Thu, 11 Oct 2018 12:22:06 -0400 (EDT) From: Imeev Mergen Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: assertion in autoincrement column References: <20180929042720.GC32712@chai> <924fdf95-65c1-46a4-f8a7-d397c81541dc@tarantool.org> <665AF75C-4542-4DE1-BFCA-F480367585A6@gmail.com> Message-ID: <6d17e4ac-1c24-59fe-1a47-8b03c0540bba@tarantool.org> Date: Thu, 11 Oct 2018 19:22:03 +0300 MIME-Version: 1.0 In-Reply-To: <665AF75C-4542-4DE1-BFCA-F480367585A6@gmail.com> Content-Type: multipart/alternative; boundary="------------235593AE6FA36E4987D31810" 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: =?UTF-8?B?0J3QuNC60LjRgtCwINCf0LXRgtGC0LjQug==?= , tarantool-patches@freelists.org, "n.pettik" Cc: Vladislav Shpilevoy This is a multi-part message in MIME format. --------------235593AE6FA36E4987D31810 Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Hello! Thank you for review! Diff between two last versions and new patch below. On 10/11/2018 03:48 PM, Никита Петтик wrote: >> 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 > Nit: cheched? propertly? > Correct version I guess should be: > ’This values wouldn’t be checked properly if it was inserted ...' Fixed. >> 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. >> + */ > How OP_FCopy is related to this ticket? > Comment to this opcodes clearly says: > > * Copy integer value of register P1 in root frame in to register P2 of current > * frame > > Mb you need to fix code generation for that query, but I doubt even that. > AFAIU initial problem lies in ticket #3735. Due to this behaviour (converting INT > to FLOAT for overflowed integers) we are trying to insert wrong value to > source table. Hence, this patch is likely to be useless. That is not right. For example (it is current test actually): CREATE TABLE t1 (s1 INT PRIMARY KEY AUTOINCREMENT, s2 CHAR); INSERT INTO t1 VALUES (1, 'a'); INSERT INTO t1 SELECT s2, s2 FROM t1; >> + 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” > Still: why ‘datatype mismatch’ and not ‘integer overflow’? > I guess from now this test belongs to > https://github.com/tarantool/tarantool/issues/3735 Removed this test and added commented issue 3735 https://github.com/tarantool/tarantool/issues/3735#issuecomment-428990257 >> + -- >> + }) >> + >> +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" >> + -- >> + }) >> + *Diff between two last versions:* diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 5862917..e98f844 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -35,6 +35,7 @@   */  #include "sqliteInt.h"  #include "tarantoolInt.h" +#include "vdbeInt.h"  #include "box/session.h"  #include "box/schema.h"  #include "bit/bit.h" @@ -669,6 +670,11 @@ sqlite3Insert(Parse * pParse,    /* Parser context */                       */                      sqlite3VdbeAddOp3(v, OP_Column, srcTab,                                j, regTmp); +                    sqlite3VdbeAddOp2(v, OP_IsNull, +                              regTmp, +                              v->nOp + 2); +                    sqlite3VdbeAddOp1(v, OP_MustBeInt, +                              regTmp);                      sqlite3VdbeAddOp2(v, OP_FCopy, regTmp,                                iRegStore);                      sqlite3VdbeChangeP3(v, -1, @@ -685,6 +691,14 @@ sqlite3Insert(Parse * pParse,    /* Parser context */                           * autoinc-ed value with select result                           * in case that result is NULL                           */ +                        sqlite3VdbeAddOp2(v, OP_IsNull, +                                  regFromSelect +                                  + j, +                                  v->nOp + 2); +                        sqlite3VdbeAddOp1(v, +                                  OP_MustBeInt, +                                  regFromSelect +                                  + j);                          sqlite3VdbeAddOp2(v, OP_FCopy,                                    regFromSelect                                    + j, @@ -714,6 +728,14 @@ sqlite3Insert(Parse * pParse,    /* Parser context */                           * autoinc-ed value with select result                           * in case that result is NULL                           */ +                        sqlite3VdbeAddOp2(v, OP_IsNull, +                                  pList->a[j]. +                                  pExpr->iTable, +                                  v->nOp + 2); +                        sqlite3VdbeAddOp1(v, +                                  OP_MustBeInt, +                                  pList->a[j]. +                                  pExpr->iTable);                          sqlite3VdbeAddOp2(v, OP_FCopy,                                    pList->a[j].                                    pExpr->iTable, diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index de18262..7c1015c 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -3736,15 +3736,10 @@ 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]; -        /* -         * 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); +        MemSetTypeFlag(pOut, MEM_Int);          pOut->u.i = pIn1->u.i;      } diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua index bd40de9..fa45c0e 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(48) +test:plan(47)  --!./tcltestrunner.lua  -- 2004 November 12 @@ -803,27 +803,15 @@ 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", +    "autoinc-gh-3670",      [[          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() *New patch:* commit 37bbfe1fe46c3242137de67b0032a769316aed5f 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. These     values wouldn’t be checked properly if it was inserted in     column with autoincrement. This patch fixes mentioned check.     Closes #3670 diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 5862917..e98f844 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -35,6 +35,7 @@   */  #include "sqliteInt.h"  #include "tarantoolInt.h" +#include "vdbeInt.h"  #include "box/session.h"  #include "box/schema.h"  #include "bit/bit.h" @@ -669,6 +670,11 @@ sqlite3Insert(Parse * pParse,    /* Parser context */                       */                      sqlite3VdbeAddOp3(v, OP_Column, srcTab,                                j, regTmp); +                    sqlite3VdbeAddOp2(v, OP_IsNull, +                              regTmp, +                              v->nOp + 2); +                    sqlite3VdbeAddOp1(v, OP_MustBeInt, +                              regTmp);                      sqlite3VdbeAddOp2(v, OP_FCopy, regTmp,                                iRegStore);                      sqlite3VdbeChangeP3(v, -1, @@ -685,6 +691,14 @@ sqlite3Insert(Parse * pParse,    /* Parser context */                           * autoinc-ed value with select result                           * in case that result is NULL                           */ +                        sqlite3VdbeAddOp2(v, OP_IsNull, +                                  regFromSelect +                                  + j, +                                  v->nOp + 2); +                        sqlite3VdbeAddOp1(v, +                                  OP_MustBeInt, +                                  regFromSelect +                                  + j);                          sqlite3VdbeAddOp2(v, OP_FCopy,                                    regFromSelect                                    + j, @@ -714,6 +728,14 @@ sqlite3Insert(Parse * pParse,    /* Parser context */                           * autoinc-ed value with select result                           * in case that result is NULL                           */ +                        sqlite3VdbeAddOp2(v, OP_IsNull, +                                  pList->a[j]. +                                  pExpr->iTable, +                                  v->nOp + 2); +                        sqlite3VdbeAddOp1(v, +                                  OP_MustBeInt, +                                  pList->a[j]. +                                  pExpr->iTable);                          sqlite3VdbeAddOp2(v, OP_FCopy,                                    pList->a[j].                                    pExpr->iTable, diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua index dda7061..fa45c0e 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(47)  --!./tcltestrunner.lua  -- 2004 November 12 @@ -801,4 +801,17 @@ test:do_test(          --      }) +-- gh-3670: Assertion with large number in autoincrement column +test:do_catchsql_test( +    "autoinc-gh-3670", +    [[ +        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() --------------235593AE6FA36E4987D31810 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit Hello! Thank you for review! Diff between two last versions
and new patch below.

On 10/11/2018 03:48 PM, Никита Петтик wrote:
commit 53a229c67b096a09f26df4b5ba73d9a90c46e38f
Author: Mergen Imeev <imeevma@gmail.com>
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

Nit: cheched? propertly?
Correct version I guess should be:
’This values wouldn’t be checked properly if it was inserted ...'
Fixed.
    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.
+         */

How OP_FCopy is related to this ticket?
Comment to this opcodes clearly says:

* Copy integer value of register P1 in root frame in to register P2 of current
* frame

Mb you need to fix code generation for that query, but I doubt even that.
AFAIU initial problem lies in ticket #3735. Due to this behaviour (converting INT
to FLOAT for overflowed integers) we are trying to insert wrong value to
source table. Hence, this patch is likely to be useless.
That is not right. For example (it is current test actually):

CREATE TABLE t1 (s1 INT PRIMARY KEY AUTOINCREMENT, s2 CHAR);
INSERT INTO t1 VALUES (1, 'a');
INSERT INTO t1 SELECT s2, s2 FROM t1; 
+        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(
         -- </autoinc-a69637.2>
     })

+-- 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;
+    ]], {
+        -- <autoinc-10.2>
+        1, "datatype mismatch”

Still: why ‘datatype mismatch’ and not ‘integer overflow’?
I guess from now this test belongs to
https://github.com/tarantool/tarantool/issues/3735
Removed this test and added commented issue 3735
https://github.com/tarantool/tarantool/issues/3735#issuecomment-428990257
+        -- </autoinc-10.2>
+    })
+
+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;
+    ]], {
+        -- <autoinc-10.2>
+        1, "datatype mismatch"
+        -- </autoinc-10.2>
+    })
+

Diff between two last versions:

diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 5862917..e98f844 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -35,6 +35,7 @@
  */
 #include "sqliteInt.h"
 #include "tarantoolInt.h"
+#include "vdbeInt.h"
 #include "box/session.h"
 #include "box/schema.h"
 #include "bit/bit.h"
@@ -669,6 +670,11 @@ sqlite3Insert(Parse * pParse,    /* Parser context */
                      */
                     sqlite3VdbeAddOp3(v, OP_Column, srcTab,
                               j, regTmp);
+                    sqlite3VdbeAddOp2(v, OP_IsNull,
+                              regTmp,
+                              v->nOp + 2);
+                    sqlite3VdbeAddOp1(v, OP_MustBeInt,
+                              regTmp);
                     sqlite3VdbeAddOp2(v, OP_FCopy, regTmp,
                               iRegStore);
                     sqlite3VdbeChangeP3(v, -1,
@@ -685,6 +691,14 @@ sqlite3Insert(Parse * pParse,    /* Parser context */
                          * autoinc-ed value with select result
                          * in case that result is NULL
                          */
+                        sqlite3VdbeAddOp2(v, OP_IsNull,
+                                  regFromSelect
+                                  + j,
+                                  v->nOp + 2);
+                        sqlite3VdbeAddOp1(v,
+                                  OP_MustBeInt,
+                                  regFromSelect
+                                  + j);
                         sqlite3VdbeAddOp2(v, OP_FCopy,
                                   regFromSelect
                                   + j,
@@ -714,6 +728,14 @@ sqlite3Insert(Parse * pParse,    /* Parser context */
                          * autoinc-ed value with select result
                          * in case that result is NULL
                          */
+                        sqlite3VdbeAddOp2(v, OP_IsNull,
+                                  pList->a[j].
+                                  pExpr->iTable,
+                                  v->nOp + 2);
+                        sqlite3VdbeAddOp1(v,
+                                  OP_MustBeInt,
+                                  pList->a[j].
+                                  pExpr->iTable);
                         sqlite3VdbeAddOp2(v, OP_FCopy,
                                   pList->a[j].
                                   pExpr->iTable,
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index de18262..7c1015c 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3736,15 +3736,10 @@ 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];
-        /*
-         * 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);
+        MemSetTypeFlag(pOut, MEM_Int);
 
         pOut->u.i = pIn1->u.i;
     }
diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua
index bd40de9..fa45c0e 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(48)
+test:plan(47)
 
 --!./tcltestrunner.lua
 -- 2004 November 12
@@ -803,27 +803,15 @@ 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;
-    ]], {
-        -- <autoinc-10.2>
-        1, "datatype mismatch"
-        -- </autoinc-10.2>
-    })
-
-test:do_catchsql_test(
-    "autoinc-gh-3670-2",
+    "autoinc-gh-3670",
     [[
         CREATE TABLE t1 (s1 INT PRIMARY KEY AUTOINCREMENT, s2 CHAR);
         INSERT INTO t1 VALUES (1, 'a');
         INSERT INTO t1 SELECT s2, s2 FROM t1;
     ]], {
-        -- <autoinc-10.2>
+        -- <autoinc-gh-3670>
         1, "datatype mismatch"
-        -- </autoinc-10.2>
+        -- </autoinc-gh-3670>
     })
 
 test:finish_test()

New patch:

commit 37bbfe1fe46c3242137de67b0032a769316aed5f
Author: Mergen Imeev <imeevma@gmail.com>
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. These
    values wouldn’t be checked properly if it was inserted in
    column with autoincrement. This patch fixes mentioned check.
   
    Closes #3670

diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 5862917..e98f844 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -35,6 +35,7 @@
  */
 #include "sqliteInt.h"
 #include "tarantoolInt.h"
+#include "vdbeInt.h"
 #include "box/session.h"
 #include "box/schema.h"
 #include "bit/bit.h"
@@ -669,6 +670,11 @@ sqlite3Insert(Parse * pParse,    /* Parser context */
                      */
                     sqlite3VdbeAddOp3(v, OP_Column, srcTab,
                               j, regTmp);
+                    sqlite3VdbeAddOp2(v, OP_IsNull,
+                              regTmp,
+                              v->nOp + 2);
+                    sqlite3VdbeAddOp1(v, OP_MustBeInt,
+                              regTmp);
                     sqlite3VdbeAddOp2(v, OP_FCopy, regTmp,
                               iRegStore);
                     sqlite3VdbeChangeP3(v, -1,
@@ -685,6 +691,14 @@ sqlite3Insert(Parse * pParse,    /* Parser context */
                          * autoinc-ed value with select result
                          * in case that result is NULL
                          */
+                        sqlite3VdbeAddOp2(v, OP_IsNull,
+                                  regFromSelect
+                                  + j,
+                                  v->nOp + 2);
+                        sqlite3VdbeAddOp1(v,
+                                  OP_MustBeInt,
+                                  regFromSelect
+                                  + j);
                         sqlite3VdbeAddOp2(v, OP_FCopy,
                                   regFromSelect
                                   + j,
@@ -714,6 +728,14 @@ sqlite3Insert(Parse * pParse,    /* Parser context */
                          * autoinc-ed value with select result
                          * in case that result is NULL
                          */
+                        sqlite3VdbeAddOp2(v, OP_IsNull,
+                                  pList->a[j].
+                                  pExpr->iTable,
+                                  v->nOp + 2);
+                        sqlite3VdbeAddOp1(v,
+                                  OP_MustBeInt,
+                                  pList->a[j].
+                                  pExpr->iTable);
                         sqlite3VdbeAddOp2(v, OP_FCopy,
                                   pList->a[j].
                                   pExpr->iTable,
diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua
index dda7061..fa45c0e 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(47)
 
 --!./tcltestrunner.lua
 -- 2004 November 12
@@ -801,4 +801,17 @@ test:do_test(
         -- </autoinc-a69637.2>
     })
 
+-- gh-3670: Assertion with large number in autoincrement column
+test:do_catchsql_test(
+    "autoinc-gh-3670",
+    [[
+        CREATE TABLE t1 (s1 INT PRIMARY KEY AUTOINCREMENT, s2 CHAR);
+        INSERT INTO t1 VALUES (1, 'a');
+        INSERT INTO t1 SELECT s2, s2 FROM t1;
+    ]], {
+        -- <autoinc-gh-3670>
+        1, "datatype mismatch"
+        -- </autoinc-gh-3670>
+    })
+
 test:finish_test()

--------------235593AE6FA36E4987D31810--