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

Kirill Shcherbatov kshcherbatov at tarantool.org
Fri Aug 31 18:45:42 MSK 2018


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





More information about the Tarantool-patches mailing list