[tarantool-patches] [PATCH v1 3/3] sql: dissallow bindings for DDL

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

Bindings could not be used in stored ACTs because they allocate
memory registers and makes assignments on parse sequentially.
Original sqlite3 did validations that persistent AST doesn't have
auto-assigment Varibles on triggers and checks creation.
On DDL integration complete we've get rid this mechanism.
Now it should be returned.

Closes #3653.
---
 src/box/space_def.c         |  3 ++-
 src/box/sql/parse.y         |  6 +++++-
 src/box/sql/tokenize.c      |  8 ++++----
 test/sql-tap/check.test.lua |  4 ++--
 test/sql/checks.result      | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 test/sql/checks.test.lua    | 18 +++++++++++++++++-
 6 files changed, 74 insertions(+), 10 deletions(-)

diff --git a/src/box/space_def.c b/src/box/space_def.c
index f5ca0b5..542289e 100644
--- a/src/box/space_def.c
+++ b/src/box/space_def.c
@@ -338,7 +338,8 @@ checks_array_decode(const char **str, uint32_t len, char *opt, uint32_t errcode,
 			box_error_t *err = box_error_last();
 			if (box_error_code(err) != ENOMEM) {
 				snprintf(errmsg, TT_STATIC_BUF_LEN,
-					 "invalid expression specified");
+					 "invalid expression specified (%s)",
+					 box_error_message(err));
 				diag_set(ClientError, errcode, field_no,
 					 errmsg);
 			}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index d8532d3..60cf3f3 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -881,7 +881,11 @@ term(A) ::= INTEGER(X). {
 }
 expr(A) ::= VARIABLE(X).     {
   Token t = X;
-  if( !(X.z[0]=='#' && sqlite3Isdigit(X.z[1])) ){
+  if (pParse->parse_only) {
+    spanSet(&A, &t, &t);
+    sqlite3ErrorMsg(pParse, "bindings are not allowed in DDL");
+    A.pExpr = NULL;
+  } else if (!(X.z[0]=='#' && sqlite3Isdigit(X.z[1]))) {
     u32 n = X.n;
     spanExpr(&A, pParse, TK_VARIABLE, X);
     if (A.pExpr->u.zToken[0] == '?' && n > 1)
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index ec06456..4eebfe5 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -561,10 +561,10 @@ sql_expr_compile(sqlite3 *db, const char *expr, int expr_len)
 	}
 	sprintf(stmt, "%s%.*s", outer, expr_len, expr);
 
-	char *unused;
-	if (sqlite3RunParser(&parser, stmt, &unused) != SQLITE_OK ||
+	char *sql_error = NULL;
+	if (sqlite3RunParser(&parser, stmt, &sql_error) != SQLITE_OK ||
 	    parser.parsed_ast_type != AST_TYPE_EXPR) {
-		diag_set(ClientError, ER_SQL_EXECUTE, stmt);
+		diag_set(ClientError, ER_SQL, sql_error);
 	} else {
 		expression = parser.parsed_ast.expr;
 		parser.parsed_ast.expr = NULL;
@@ -602,7 +602,7 @@ sql_trigger_compile(struct sqlite3 *db, const char *sql)
 	struct Parse parser;
 	sql_parser_create(&parser, db);
 	parser.parse_only = true;
-	char *sql_error;
+	char *sql_error = NULL;
 	struct sql_trigger *trigger = NULL;
 	if (sqlite3RunParser(&parser, sql, &sql_error) != SQLITE_OK ||
 	    parser.parsed_ast_type != AST_TYPE_TRIGGER) {
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index ff36552..f03ac7b 100755
--- a/test/sql-tap/check.test.lua
+++ b/test/sql-tap/check.test.lua
@@ -555,7 +555,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <check-5.1>
-        1, "Failed to create space 'T5': SQL error: parameters prohibited in CHECK constraints"
+        1, "Wrong space options (field 5): invalid expression specified (SQL error: bindings are not allowed in DDL)"
         -- </check-5.1>
     })
 
@@ -567,7 +567,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <check-5.2>
-        1, "Failed to create space 'T5': SQL error: parameters prohibited in CHECK constraints"
+        1, "Wrong space options (field 5): invalid expression specified (SQL error: bindings are not allowed in DDL)"
         -- </check-5.2>
     })
 
diff --git a/test/sql/checks.result b/test/sql/checks.result
index 3084d89..a88e048 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -29,7 +29,8 @@ t = {513, 1, 'test', 'memtx', 0, opts, format}
 ...
 s = box.space._space:insert(t)
 ---
-- error: 'Wrong space options (field 5): invalid expression specified'
+- error: 'Wrong space options (field 5): invalid expression specified (SQL error:
+    near "<": syntax error)'
 ...
 opts = {checks = {{expr = 'X>5'}}}
 ---
@@ -116,6 +117,48 @@ box.sql.execute("DROP TABLE w2;")
 ---
 - error: 'no such table: W2'
 ...
+--
+-- gh-3653: Dissallow bindings for DDL
+--
+box.sql.execute("CREATE TABLE t1(a INT PRIMARY KEY, b INT);")
+---
+...
+space_id = box.space.T1.id
+---
+...
+box.sql.execute("CREATE TRIGGER tr1 AFTER INSERT ON t1 WHEN new.a = ? BEGIN SELECT 1; END;")
+---
+- error: 'SQL error: bindings are not allowed in DDL'
+...
+tuple = {"TR1", space_id, {sql = [[CREATE TRIGGER tr1 AFTER INSERT ON t1 WHEN new.a = ? BEGIN SELECT 1; END;]]}}
+---
+...
+box.space._trigger:insert(tuple)
+---
+- error: 'SQL error: bindings are not allowed in DDL'
+...
+box.sql.execute("DROP TABLE t1;")
+---
+...
+box.sql.execute("CREATE TABLE t5(x primary key, y,CHECK( x*y<? ));")
+---
+- error: 'Wrong space options (field 5): invalid expression specified (SQL error:
+    bindings are not allowed in DDL)'
+...
+opts = {checks = {{expr = '?>5', name = 'ONE'}}}
+---
+...
+format = {{name = 'X', type = 'unsigned'}}
+---
+...
+t = {513, 1, 'test', 'memtx', 0, opts, format}
+---
+...
+s = box.space._space:insert(t)
+---
+- error: 'Wrong space options (field 5): invalid expression specified (SQL error:
+    bindings are not allowed in DDL)'
+...
 test_run:cmd("clear filter")
 ---
 - true
diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua
index fb95809..3506d5c 100644
--- a/test/sql/checks.test.lua
+++ b/test/sql/checks.test.lua
@@ -43,11 +43,27 @@ format = {{name = 'X', type = 'unsigned'}}
 t = {513, 1, 'test', 'memtx', 0, opts, format}
 s = box.space._space:insert(t)
 
-
 --
 -- gh-3611: Segfault on table creation with check referencing this table
 --
 box.sql.execute("CREATE TABLE w2 (s1 INT PRIMARY KEY, CHECK ((SELECT COUNT(*) FROM w2) = 0));")
 box.sql.execute("DROP TABLE w2;")
 
+--
+-- gh-3653: Dissallow bindings for DDL
+--
+box.sql.execute("CREATE TABLE t1(a INT PRIMARY KEY, b INT);")
+space_id = box.space.T1.id
+box.sql.execute("CREATE TRIGGER tr1 AFTER INSERT ON t1 WHEN new.a = ? BEGIN SELECT 1; END;")
+tuple = {"TR1", space_id, {sql = [[CREATE TRIGGER tr1 AFTER INSERT ON t1 WHEN new.a = ? BEGIN SELECT 1; END;]]}}
+box.space._trigger:insert(tuple)
+box.sql.execute("DROP TABLE t1;")
+
+box.sql.execute("CREATE TABLE t5(x primary key, y,CHECK( x*y<? ));")
+
+opts = {checks = {{expr = '?>5', name = 'ONE'}}}
+format = {{name = 'X', type = 'unsigned'}}
+t = {513, 1, 'test', 'memtx', 0, opts, format}
+s = box.space._space:insert(t)
+
 test_run:cmd("clear filter")
-- 
2.7.4