Tarantool development patches archive
 help / color / mirror / Atom feed
From: Imeev Mergen <imeevma@tarantool.org>
To: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: assertion in autoincrement column
Date: Fri, 5 Oct 2018 19:41:21 +0300	[thread overview]
Message-ID: <924fdf95-65c1-46a4-f8a7-d397c81541dc@tarantool.org> (raw)
In-Reply-To: <20180929042720.GC32712@chai>

Hello! Thank you for review! New patch below.


On 09/29/2018 07:27 AM, Konstantin Osipov wrote:
> * imeevma@tarantool.org <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 <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
     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(
          -- </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"
+        -- </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>
+    })
+
  test:finish_test()

  reply	other threads:[~2018-10-05 16:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-28 16:14 [tarantool-patches] " imeevma
2018-09-29  4:27 ` [tarantool-patches] " Konstantin Osipov
2018-10-05 16:41   ` Imeev Mergen [this message]
2018-10-05 16:45     ` Imeev Mergen
     [not found]     ` <665AF75C-4542-4DE1-BFCA-F480367585A6@gmail.com>
2018-10-11 16:22       ` Imeev Mergen
2018-10-11 16:38         ` n.pettik
2018-10-03 15:19 ` Vladislav Shpilevoy
2018-10-05 16:40   ` Imeev Mergen
2018-10-10 10:21     ` Vladislav Shpilevoy
2018-10-10 12:22       ` Imeev Mergen
2018-11-01 14:40 ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=924fdf95-65c1-46a4-f8a7-d397c81541dc@tarantool.org \
    --to=imeevma@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v1 1/1] sql: assertion in autoincrement column' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox