[tarantool-patches] [PATCH] sql: disallow id duplicate in insert & update

[AKhatskevich <avkhatskevich@tarantool.org>]

Before this patch one was able to exec query like this:
  `insert into t(col1, col1) values(1, 1)`
  `update t set col1 = 1, col1 = 2;`
which is nonsense.

New policy is to throw an error in case of duplicates in columns
description.

Closes #2358
---
 src/box/sql/insert.c          | 15 +++++++++++++++
 src/box/sql/sqliteLimit.h     |  9 +++++++++
 src/box/sql/update.c          |  7 +++++++
 test/sql-tap/insert1.test.lua | 30 +++++++++++++++++++++++++++++-
 test/sql-tap/update.test.lua  | 23 ++++++++++++++++++++++-
 5 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 42254ddf4..33175e54c 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -35,6 +35,7 @@
  */
 #include "sqliteInt.h"
 #include "box/session.h"
+#include "bit/bit.h"
 
 /*
  * Generate code that will open pTab as cursor iCur.
@@ -450,6 +451,12 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 	 * is appears in the original table.  (The index of the INTEGER
 	 * PRIMARY KEY in the original table is pTab->iPKey.)
 	 */
+	/* Create bitmask to mark used columns of the table. */
+	void *used_columns = tt_static_buf();
+	/* The size of used_columns buffer is checked during compilation time
+	 * using SQLITE_MAX_COLUMN constant.
+	 */
+	memset(used_columns, 0, (pTab->nCol + 7) / 8);
 	bIdListInOrder = 1;
 	if (pColumn) {
 		for (i = 0; i < pColumn->nId; i++) {
@@ -477,6 +484,14 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 				pParse->checkSchema = 1;
 				goto insert_cleanup;
 			}
+			if (bit_test(used_columns, j)) {
+				const char *err;
+				err = "table id list: duplicate column name %s";
+				sqlite3ErrorMsg(pParse,
+						err, pColumn->a[i].zName);
+				goto insert_cleanup;
+			}
+			bit_set(used_columns, j);
 		}
 	}
 
diff --git a/src/box/sql/sqliteLimit.h b/src/box/sql/sqliteLimit.h
index 138268259..f6b580665 100644
--- a/src/box/sql/sqliteLimit.h
+++ b/src/box/sql/sqliteLimit.h
@@ -35,6 +35,7 @@
  *
  * This file defines various limits of what SQLite can process.
  */
+#include "trivia/util.h"
 
 enum {
 	/*
@@ -74,6 +75,14 @@ enum {
 #ifndef SQLITE_MAX_COLUMN
 #define SQLITE_MAX_COLUMN 2000
 #endif
+/*
+ * tt_static_buf() is used to store bitmask for used columns in a table during
+ * SQL parsing stage. The following statement checks if static buffer is big
+ * enough to store the bitmask.
+ */
+#if SQLITE_MAX_COLUMN > TT_STATIC_BUF_LEN * 8
+#error "Bitmask for used table columns cannot fit into static buffer"
+#endif
 
 /*
  * The maximum length of a single SQL statement in bytes.
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index bf413252d..91717d7bd 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -233,6 +233,13 @@ sqlite3Update(Parse * pParse,		/* The parser context */
 				if (pPk && table_column_is_in_pk(pTab, j)) {
 					chngPk = 1;
 				}
+				if (aXRef[j] != -1) {
+					sqlite3ErrorMsg(pParse,
+							"set id list: duplicate"
+							" column name %s",
+							pChanges->a[i].zName);
+					goto update_cleanup;
+				}
 				aXRef[j] = i;
 				break;
 			}
diff --git a/test/sql-tap/insert1.test.lua b/test/sql-tap/insert1.test.lua
index 847b25ad2..750732d37 100755
--- a/test/sql-tap/insert1.test.lua
+++ b/test/sql-tap/insert1.test.lua
@@ -1,7 +1,7 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
 yaml = require('yaml')
-test:plan(24)
+test:plan(28)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -516,5 +516,33 @@ test:do_execsql_test("insert-4.7", [[
 --   SELECT * FROM t12c;
 -- } {one xyzzy two}
 -- integrity_check insert-99.0
+
+
+test:do_execsql_test(
+    "insert-13.0",
+    [[
+        create table test(a primary key, b)
+    ]])
+
+test:do_catchsql_test(
+    "insert-13.1",
+    [[
+        insert into test(a, a, b) values(1, 1, 1)
+    ]],
+    {1, "table id list: duplicate column name A"})
+
+test:do_catchsql_test(
+    "insert-13.2",
+    [[
+        insert into test(a, b, b) values(1, 1, 1)
+    ]],
+    {1, "table id list: duplicate column name B"})
+
+test:do_execsql_test(
+    "insert-13.3",
+    [[
+        drop table test;
+    ]])
+
 test:finish_test()
 
diff --git a/test/sql-tap/update.test.lua b/test/sql-tap/update.test.lua
index f6e756731..a4efe4015 100755
--- a/test/sql-tap/update.test.lua
+++ b/test/sql-tap/update.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(108)
+test:plan(111)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -1109,5 +1109,26 @@ test:do_catchsql_test("update-10.10", [[
 --   UPDATE t15 SET c=printf("y%d",a) WHERE c IS NULL;
 --   SELECT a,b,c,'|' FROM t15 ORDER BY a;
 -- } {5 zyx y5 | 10 abc y10 | 15 wvu y15 | 20 def y20 | 25 tsr y25 | 30 ghi y30 | 35 qpo y35 |}
+
+test:do_execsql_test(
+    "insert-15.0",
+    [[
+        create table test(a primary key);
+        insert into test(a) values(1);
+    ]])
+
+test:do_catchsql_test(
+    "insert-15.1",
+    [[
+        update test set a = 2, a = 3;
+    ]],
+    {1, "set id list: duplicate column name A"})
+
+test:do_execsql_test(
+  "insert-15.2",
+  [[
+      drop table test;
+  ]])
+
 test:finish_test()
 
-- 
2.14.1