From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 3C89D29170 for ; Fri, 31 Aug 2018 11:45:51 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id e_-r_xCvtIf5 for ; Fri, 31 Aug 2018 11:45:51 -0400 (EDT) Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 6C4CD29199 for ; Fri, 31 Aug 2018 11:45:50 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v1 3/3] sql: dissallow bindings for DDL Date: Fri, 31 Aug 2018 18:45:42 +0300 Message-Id: <5049d3e7b70b7091c51ac99fc64f14a07c879c8a.1535730218.git.kshcherbatov@tarantool.org> In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: korablev@tarantool.org, Kirill Shcherbatov 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( ); ]], { -- - 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)" -- }) @@ -567,7 +567,7 @@ test:do_catchsql_test( ); ]], { -- - 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)" -- }) 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*y5', 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*y5', 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