[patches] [PATCH 1/2] sql: ban multiple ON CONFLICT REPLACE

[Bulat Niatshin]

Ban an opportunity for user to create table with multiple constraints
with ON CONFLICT REPLACE option.

For #2963
---
 src/box/sql/build.c                            | 53 ++++++++++++++++++++
 test/sql-tap/gh-2963-multiple-replace.test.lua | 69 ++++++++++++++++++++++++++
 2 files changed, 122 insertions(+)
 create mode 100755 test/sql-tap/gh-2963-multiple-replace.test.lua

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index eb51a9863..9b7afb1ba 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1822,6 +1822,51 @@ emitNewSysSpaceSequenceRecord(Parse *pParse, int space_id, const char reg_seq_id
 	return first_col;
 }
 
+/* This function will be called during execution of sqlite3EndTable.
+ * It will ensure that only one NOT NULL and UNIQUE constraints
+ * with ON CONFLICT REPLACE clause are specified by user
+ * (true will be returned in that case), otherwise
+ * false will be returned and appropriate error message will be showed.
+ */
+static bool
+check_multiple_replace_entries(Table * table)
+{
+	bool on_replace_used = false;
+	Index * idx;
+	int i;
+
+	/* Check all UNIQUE constraints (which are represented by unique
+	 * indexes) on the subject of one specified
+	 * ON CONFLICT REPLACE clause.
+	 */
+	for (idx = table->pIndex; idx; idx = idx->pNext) {
+		if (idx->onError == OE_Replace) {
+			if (on_replace_used == true) {
+				return true;
+			}
+			on_replace_used = true;
+		}
+	}
+
+	on_replace_used = false;
+
+	/* Check all NOT NULL constraints. Iterate through columns, because
+	 * all info about NOT NULL is stored inside colun structure.
+	 *
+	 */
+	for (i = 0; i < table->nCol; i++) {
+		u8 on_error = table->aCol[i].notNull;
+		if (on_error == OE_Replace) {
+			if (on_replace_used == true) {
+				return true;
+			}
+			on_replace_used = true;
+		}
+	}
+
+	return false;
+}
+
 /*
  * This routine is called to report the final ")" that terminates
  * a CREATE TABLE statement.
@@ -1884,6 +1929,14 @@ sqlite3EndTable(Parse * pParse,	/* Parse context */
 		}
 	}
 
+	if (check_multiple_replace_entries(p)) {
+		sqlite3ErrorMsg(pParse,
+				"Table %s can feature only one "
+				"ON CONFLICT REPLACE constraint",
+				p->zName);
+		return;
+	}
+
 #ifndef SQLITE_OMIT_CHECK
 	/* Resolve names in all CHECK constraint expressions.
 	 */
diff --git a/test/sql-tap/gh-2963-multiple-replace.test.lua b/test/sql-tap/gh-2963-multiple-replace.test.lua
new file mode 100755
index 000000000..0b907173a
--- /dev/null
+++ b/test/sql-tap/gh-2963-multiple-replace.test.lua
@@ -0,0 +1,69 @@
+#!/usr/bin/env tarantool
+-- In SQLite multiple ON CONFLICT REPLACE were allowed in CREATE TABLE statement.
+-- However, we decided that we should have only one constraint with ON CONFLICT
+-- REPLACE action. This tests check that proper error message will be raised
+-- if user makes multiple columns with ON CONFLICT REPLACE.
+
+test = require("sqltester")
+test:plan(5)
+
+test:do_execsql_test(
+    "replace-1.1",
+    [[
+        CREATE TABLE t1(a INT PRIMARY KEY,
+                        b INT UNIQUE ON CONFLICT REPLACE,
+                        c INT UNIQUE
+                        );
+    ]], {
+
+    })
+
+test:do_catchsql_test(
+    "replace-1.2",
+    [[
+        DROP TABLE IF EXISTS t1;
+        CREATE TABLE t1(a INT PRIMARY KEY,
+                        b INT UNIQUE ON CONFLICT REPLACE,
+                        c INT UNIQUE ON CONFLICT REPLACE
+                        );
+    ]], {
+        1, "Table T1 can feature only one ON CONFLICT REPLACE constraint"
+    })
+
+test:do_execsql_test(
+    "replace-1.3",
+    [[
+        DROP TABLE IF EXISTS t1;
+        CREATE TABLE t1(a INT PRIMARY KEY,
+                        b INT UNIQUE ON CONFLICT REPLACE,
+                        c INT NOT NULL ON CONFLICT REPLACE
+                        );
+    ]], {
+
+    })
+
+test:do_execsql_test(
+    "replace-1.4",
+    [[
+        DROP TABLE IF EXISTS t1;
+        CREATE TABLE t1(a INT PRIMARY KEY,
+                        b INT NOT NULL ON CONFLICT REPLACE,
+                        c INT UNIQUE ON CONFLICT REPLACE
+                        );
+    ]], {
+
+    })
+
+test:do_catchsql_test(
+    "replace-1.5",
+    [[
+        DROP TABLE IF EXISTS t1;
+        CREATE TABLE t1(a INT PRIMARY KEY,
+                        b INT NOT NULL ON CONFLICT REPLACE,
+                        c INT NOT NULL ON CONFLICT REPLACE
+                        );
+    ]], {
+        1, "Table T1 can feature only one ON CONFLICT REPLACE constraint"
+    })
+
+test:finish_test()
-- 
2.14.1