* [tarantool-patches] [PATCH v1 1/6] box: rename space->opts checks to checks_ast
2018-11-29 13:09 [tarantool-patches] [PATCH v1 0/6] sql: Checks on server side Kirill Shcherbatov
@ 2018-11-29 13:09 ` Kirill Shcherbatov
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 2/6] sql: disallow use of TYPEOF in Check Kirill Shcherbatov
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Kirill Shcherbatov @ 2018-11-29 13:09 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
The checks field of space_opts object renamed to checks_ast to avoid a
confusion due to introduction of a new object sql_check, representing
precompiled VDBE program to execute.
Need for #3691
---
src/box/alter.cc | 4 ++--
src/box/space_def.c | 15 ++++++++-------
src/box/space_def.h | 4 ++--
src/box/sql.c | 2 +-
src/box/sql/build.c | 17 +++++++++--------
5 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 029da029e..3e9b43a54 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -540,8 +540,8 @@ space_def_new_from_tuple(struct tuple *tuple, uint32_t errcode,
space_def_new_xc(id, uid, exact_field_count, name, name_len,
engine_name, engine_name_len, &opts, fields,
field_count);
- if (def->opts.checks != NULL &&
- sql_checks_resolve_space_def_reference(def->opts.checks,
+ if (def->opts.checks_ast != NULL &&
+ sql_checks_resolve_space_def_reference(def->opts.checks_ast,
def) != 0) {
box_error_t *err = box_error_last();
if (box_error_code(err) != ENOMEM) {
diff --git a/src/box/space_def.c b/src/box/space_def.c
index 69b92c5c1..585d29e1d 100644
--- a/src/box/space_def.c
+++ b/src/box/space_def.c
@@ -55,7 +55,7 @@ const struct space_opts space_opts_default = {
/* .is_temporary = */ false,
/* .view = */ false,
/* .sql = */ NULL,
- /* .checks = */ NULL,
+ /* .checks_ast = */ NULL,
};
const struct opt_def space_opts_reg[] = {
@@ -63,7 +63,7 @@ const struct opt_def space_opts_reg[] = {
OPT_DEF("temporary", OPT_BOOL, struct space_opts, is_temporary),
OPT_DEF("view", OPT_BOOL, struct space_opts, is_view),
OPT_DEF("sql", OPT_STRPTR, struct space_opts, sql),
- OPT_DEF_ARRAY("checks", struct space_opts, checks,
+ OPT_DEF_ARRAY("checks", struct space_opts, checks_ast,
checks_array_decode),
OPT_END,
};
@@ -112,15 +112,16 @@ space_def_dup_opts(struct space_def *def, const struct space_opts *opts)
return -1;
}
}
- if (opts->checks != NULL) {
- def->opts.checks = sql_expr_list_dup(sql_get(), opts->checks, 0);
- if (def->opts.checks == NULL) {
+ if (opts->checks_ast != NULL) {
+ def->opts.checks_ast =
+ sql_expr_list_dup(sql_get(), opts->checks_ast, 0);
+ if (def->opts.checks_ast == NULL) {
free(def->opts.sql);
diag_set(OutOfMemory, 0, "sql_expr_list_dup",
"def->opts.checks");
return -1;
}
- sql_checks_update_space_def_reference(def->opts.checks, def);
+ sql_checks_update_space_def_reference(def->opts.checks_ast, def);
}
return 0;
}
@@ -283,7 +284,7 @@ void
space_opts_destroy(struct space_opts *opts)
{
free(opts->sql);
- sql_expr_list_delete(sql_get(), opts->checks);
+ sql_expr_list_delete(sql_get(), opts->checks_ast);
TRASH(opts);
}
diff --git a/src/box/space_def.h b/src/box/space_def.h
index 0d1e90233..a9a119e6d 100644
--- a/src/box/space_def.h
+++ b/src/box/space_def.h
@@ -66,8 +66,8 @@ struct space_opts {
bool is_view;
/** SQL statement that produced this space. */
char *sql;
- /** SQL Checks expressions list. */
- struct ExprList *checks;
+ /** Checks AST expressions list. */
+ struct ExprList *checks_ast;
};
extern const struct space_opts space_opts_default;
diff --git a/src/box/sql.c b/src/box/sql.c
index 7b41c9926..2b710fab6 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1204,7 +1204,7 @@ sql_encode_table_opts(struct region *region, struct Table *table,
struct ExprList_item *a;
if (table != NULL) {
is_view = table->def->opts.is_view;
- struct ExprList *checks = table->def->opts.checks;
+ struct ExprList *checks = table->def->opts.checks_ast;
if (checks != NULL) {
checks_cnt = checks->nExpr;
a = checks->a;
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 52f0bde15..7d32f3874 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -276,7 +276,7 @@ table_delete(struct sqlite3 *db, struct Table *tab)
for (uint32_t i = 0; i < tab->space->index_count; ++i)
index_def_delete(tab->space->index[i]->def);
/* Do not delete table->def allocated on region. */
- sql_expr_list_delete(db, tab->def->opts.checks);
+ sql_expr_list_delete(db, tab->def->opts.checks_ast);
} else if (tab->def->id == 0) {
space_def_delete(tab->def);
}
@@ -863,15 +863,16 @@ sql_add_check_constraint(struct Parse *parser, struct ExprSpan *span)
(int)(span->zEnd - span->zStart));
if (expr->u.zToken == NULL)
goto release_expr;
- table->def->opts.checks =
+ table->def->opts.checks_ast =
sql_expr_list_append(parser->db,
- table->def->opts.checks, expr);
- if (table->def->opts.checks == NULL) {
+ table->def->opts.checks_ast, expr);
+ if (table->def->opts.checks_ast == NULL) {
sqlite3DbFree(parser->db, expr->u.zToken);
goto release_expr;
}
if (parser->constraintName.n) {
- sqlite3ExprListSetName(parser, table->def->opts.checks,
+ sqlite3ExprListSetName(parser,
+ table->def->opts.checks_ast,
&parser->constraintName, 1);
}
} else {
@@ -947,7 +948,7 @@ space_checks_expr_list(uint32_t space_id)
space = space_by_id(space_id);
assert(space != NULL);
assert(space->def != NULL);
- return space->def->opts.checks;
+ return space->def->opts.checks_ast;
}
int
@@ -1610,8 +1611,8 @@ sqlite3EndTable(Parse * pParse, /* Parse context */
vdbe_emit_fkey_create(pParse, fk);
}
cleanup:
- sql_expr_list_delete(db, p->def->opts.checks);
- p->def->opts.checks = NULL;
+ sql_expr_list_delete(db, p->def->opts.checks_ast);
+ p->def->opts.checks_ast = NULL;
}
void
--
2.19.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [tarantool-patches] [PATCH v1 2/6] sql: disallow use of TYPEOF in Check
2018-11-29 13:09 [tarantool-patches] [PATCH v1 0/6] sql: Checks on server side Kirill Shcherbatov
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 1/6] box: rename space->opts checks to checks_ast Kirill Shcherbatov
@ 2018-11-29 13:09 ` Kirill Shcherbatov
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 3/6] box: exported sql_bind structure and API Kirill Shcherbatov
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Kirill Shcherbatov @ 2018-11-29 13:09 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
Due to the fact that we are going to perform CHECKs validations
on the server side, checks are performed on the fields with
affinity already applied that may differ with type of original
data.
After the introduction of static types, the need for type checks
based on the CHECKs disappeared.
Need for #3691
---
src/box/sql/build.c | 23 ++++++++++
test/sql-tap/check.test.lua | 89 ++-----------------------------------
2 files changed, 26 insertions(+), 86 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 7d32f3874..4598087ae 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -852,12 +852,35 @@ primary_key_exit:
return;
}
+static int
+sql_check_expr_test_callback(struct Walker *walker, struct Expr *expr)
+{
+ if (expr->op == TK_FUNCTION && strcmp(expr->u.zToken, "TYPEOF") == 0) {
+ diag_set(ClientError, ER_SQL, "TYPEOF function is forbidden "
+ "for usage in Check constraint");
+ walker->eCode = 1;
+ return WRC_Abort;
+ }
+ return WRC_Continue;
+}
+
void
sql_add_check_constraint(struct Parse *parser, struct ExprSpan *span)
{
struct Expr *expr = span->pExpr;
struct Table *table = parser->pNewTable;
if (table != NULL) {
+ /* Test if Check expression valid. */
+ struct Walker w;
+ memset(&w, 0, sizeof(w));
+ w.xExprCallback = sql_check_expr_test_callback;
+ sqlite3WalkExpr(&w, expr);
+ if (w.eCode != 0) {
+ parser->rc = SQL_TARANTOOL_ERROR;
+ parser->nErr++;
+ goto release_expr;
+ }
+
expr->u.zToken =
sqlite3DbStrNDup(parser->db, (char *)span->zStart,
(int)(span->zEnd - span->zStart));
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index c24ae2ff1..e78f06b3b 100755
--- a/test/sql-tap/check.test.lua
+++ b/test/sql-tap/check.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(58)
+test:plan(51)
--!./tcltestrunner.lua
-- 2005 November 2
@@ -201,7 +201,7 @@ test:do_execsql_test(
-- </check-1.17>
})
-test:do_execsql_test(
+test:do_catchsql_test(
"check-2.1",
[[
CREATE TABLE t2(
@@ -212,93 +212,10 @@ test:do_execsql_test(
);
]], {
-- <check-2.1>
-
+ 1, "SQL error: TYPEOF function is forbidden for usage in Check constraint"
-- </check-2.1>
})
-test:do_execsql_test(
- "check-2.2",
- [[
- INSERT INTO t2 VALUES(1, 1,2.2,'three');
- SELECT x, y, z FROM t2;
- ]], {
- -- <check-2.2>
- 1, 2.2, "three"
- -- </check-2.2>
- })
-
---db("close")
---sqlite3("db", "test.db")
-test:do_execsql_test(
- "check-2.3",
- [[
- INSERT INTO t2 VALUES(2, NULL, NULL, NULL);
- SELECT x, y, z FROM t2;
- ]], {
- -- <check-2.3>
- 1, 2.2, "three", "", "", ""
- -- </check-2.3>
- })
-
-test:do_catchsql_test(
- "check-2.4",
- [[
- INSERT INTO t2 VALUES(3, 1.1, NULL, NULL);
- ]], {
- -- <check-2.4>
- 1, "CHECK constraint failed: ONE"
- -- </check-2.4>
- })
-
-test:do_catchsql_test(
- "check-2.5",
- [[
- INSERT INTO t2 VALUES(4, NULL, 5, NULL);
- ]], {
- -- <check-2.5>
- 1, "CHECK constraint failed: TWO"
- -- </check-2.5>
- })
-
-test:do_catchsql_test(
- "check-2.6",
- [[
- INSERT INTO t2 VALUES(5, NULL, NULL, 3.14159);
- ]], {
- -- <check-2.6>
- 1, "CHECK constraint failed: THREE"
- -- </check-2.6>
- })
-
--- gh-3504: Check the CONSTRAINT name clause can't follow a constraint.
-
-test:do_catchsql_test(
- "check-2.10",
- [[
- CREATE TABLE t2b(
- x INTEGER CHECK( typeof(coalesce(x,0))=='integer' ) CONSTRAINT one,
- PRIMARY KEY (x)
- );
- ]], {
- -- <check-2.10>
- 1,"near \",\": syntax error"
- -- </check-2.10>
- })
-
-test:do_catchsql_test(
- "check-2.11",
- [[
- CREATE TABLE t2c(
- x INTEGER CONSTRAINT one CHECK( typeof(coalesce(x,0))=='integer' )
- CONSTRAINT two,
- PRIMARY KEY (x)
- );
- ]], {
- -- <check-2.10>
- 1,"near \",\": syntax error"
- -- </check-2.10>
- })
-
test:do_execsql_test(
"check-2.cleanup",
[[
--
2.19.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [tarantool-patches] [PATCH v1 3/6] box: exported sql_bind structure and API
2018-11-29 13:09 [tarantool-patches] [PATCH v1 0/6] sql: Checks on server side Kirill Shcherbatov
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 1/6] box: rename space->opts checks to checks_ast Kirill Shcherbatov
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 2/6] sql: disallow use of TYPEOF in Check Kirill Shcherbatov
@ 2018-11-29 13:09 ` Kirill Shcherbatov
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 4/6] sql: release OP_Idx{Insert,Replace} p2 argument Kirill Shcherbatov
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Kirill Shcherbatov @ 2018-11-29 13:09 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
We need exprort sql_bind structure, sql_bind_decode and
sql_bind_column routines to make SQL Vars bindings for Vdbe code
outside of execute module, preparing Checks stmt for execution.
Need for #3691
---
src/box/execute.c | 48 ++-----------------------------------------
src/box/execute.h | 52 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+), 46 deletions(-)
diff --git a/src/box/execute.c b/src/box/execute.c
index f88572d22..35833832e 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -57,31 +57,6 @@ const char *sql_info_key_strs[] = {
"row count",
};
-/**
- * Name and value of an SQL prepared statement parameter.
- * @todo: merge with sqlite3_value.
- */
-struct sql_bind {
- /** Bind name. NULL for ordinal binds. */
- const char *name;
- /** Length of the @name. */
- uint32_t name_len;
- /** Ordinal position of the bind, for ordinal binds. */
- uint32_t pos;
-
- /** Byte length of the value. */
- uint32_t bytes;
- /** SQL type of the value. */
- uint8_t type;
- /** Bind value. */
- union {
- double d;
- int64_t i64;
- /** For string or blob. */
- const char *s;
- };
-};
-
/**
* Return a string name of a parameter marker.
* @param Bind to get name.
@@ -96,17 +71,7 @@ sql_bind_name(const struct sql_bind *bind)
return tt_sprintf("%d", (int) bind->pos);
}
-/**
- * Decode a single bind column from the binary protocol packet.
- * @param[out] bind Bind to decode to.
- * @param i Ordinal bind number.
- * @param packet MessagePack encoded parameter value. Either
- * scalar or map: {string_name: scalar_value}.
- *
- * @retval 0 Success.
- * @retval -1 Memory or client error.
- */
-static inline int
+int
sql_bind_decode(struct sql_bind *bind, int i, const char **packet)
{
bind->pos = i + 1;
@@ -432,16 +397,7 @@ error:
return -1;
}
-/**
- * Bind SQL parameter value to its position.
- * @param stmt Prepared statement.
- * @param p Parameter value.
- * @param pos Ordinal bind position.
- *
- * @retval 0 Success.
- * @retval -1 SQL error.
- */
-static inline int
+int
sql_bind_column(struct sqlite3_stmt *stmt, const struct sql_bind *p,
uint32_t pos)
{
diff --git a/src/box/execute.h b/src/box/execute.h
index 77bfd7913..a48de9319 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -52,6 +52,7 @@ struct obuf;
struct region;
struct sql_bind;
struct xrow_header;
+struct sqlite3_stmt;
/** EXECUTE request. */
struct sql_request {
@@ -137,6 +138,57 @@ int
sql_prepare_and_execute(const struct sql_request *request,
struct sql_response *response, struct region *region);
+/**
+ * Name and value of an SQL prepared statement parameter.
+ * @todo: merge with sqlite3_value.
+ */
+struct sql_bind {
+ /** Bind name. NULL for ordinal binds. */
+ const char *name;
+ /** Length of the @name. */
+ uint32_t name_len;
+ /** Ordinal position of the bind, for ordinal binds. */
+ uint32_t pos;
+
+ /** Byte length of the value. */
+ uint32_t bytes;
+ /** SQL type of the value. */
+ uint8_t type;
+ /** Bind value. */
+ union {
+ double d;
+ int64_t i64;
+ /** For string or blob. */
+ const char *s;
+ };
+};
+
+/**
+ * Decode a single bind column from the binary protocol packet.
+ * @param[out] bind Bind to decode to.
+ * @param i Ordinal bind number.
+ * @param packet MessagePack encoded parameter value. Either
+ * scalar or map: {string_name: scalar_value}.
+ *
+ * @retval 0 Success.
+ * @retval -1 Memory or client error.
+ */
+int
+sql_bind_decode(struct sql_bind *bind, int i, const char **packet);
+
+/**
+ * Bind SQL parameter value to its position.
+ * @param stmt Prepared statement.
+ * @param p Parameter value.
+ * @param pos Ordinal bind position.
+ *
+ * @retval 0 Success.
+ * @retval -1 SQL error.
+ */
+int
+sql_bind_column(struct sqlite3_stmt *stmt, const struct sql_bind *p,
+ uint32_t pos);
+
#if defined(__cplusplus)
} /* extern "C" { */
#include "diag.h"
--
2.19.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [tarantool-patches] [PATCH v1 4/6] sql: release OP_Idx{Insert,Replace} p2 argument
2018-11-29 13:09 [tarantool-patches] [PATCH v1 0/6] sql: Checks on server side Kirill Shcherbatov
` (2 preceding siblings ...)
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 3/6] box: exported sql_bind structure and API Kirill Shcherbatov
@ 2018-11-29 13:09 ` Kirill Shcherbatov
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 5/6] sql: fix incomplete UPDATE on IdxInsert failured Kirill Shcherbatov
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 6/6] sql: make sql checks on server side Kirill Shcherbatov
5 siblings, 0 replies; 9+ messages in thread
From: Kirill Shcherbatov @ 2018-11-29 13:09 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
Start using p3 argument instead of p2 argument in OP_Insert and
OP_Replace opcodes. The p2 register is typically used for jump
to label and resolveP2Values() is able to patch it. As we
going to support interrupt handler for OP_Insert and OP_Replace
instructions, we need a free p2 register to reuse it by
appointment.
Need for #3691
---
src/box/sql/delete.c | 2 +-
src/box/sql/expr.c | 4 ++--
src/box/sql/insert.c | 2 +-
src/box/sql/select.c | 33 +++++++++++++++++++++------------
src/box/sql/update.c | 2 +-
src/box/sql/vdbe.c | 10 +++++-----
src/box/sql/where.c | 2 +-
src/box/sql/wherecode.c | 5 +++--
8 files changed, 35 insertions(+), 25 deletions(-)
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index f9c42fdec..9bb38abe8 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -342,7 +342,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
* by malloc.
*/
sqlite3VdbeChangeP5(v, 1);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, reg_key, reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, reg_key, 0, reg_eph);
}
/* If this DELETE cannot use the ONEPASS strategy,
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index b67b22c23..64a13bbb8 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2864,8 +2864,8 @@ sqlite3CodeSubselect(Parse * pParse, /* Parsing context */
1, r2, &affinity, 1);
sqlite3ExprCacheAffinityChange(pParse,
r3, 1);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, r2,
- reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, r2,
+ 0, reg_eph);
}
sqlite3ReleaseTempReg(pParse, r1);
sqlite3ReleaseTempReg(pParse, r2);
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 6b76bb6da..b099178d7 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -492,7 +492,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
nColumn + 1, regRec);
/* Set flag to save memory allocating one by malloc. */
sqlite3VdbeChangeP5(v, 1);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, regRec, reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, regRec, 0, reg_eph);
sqlite3VdbeGoto(v, addrL);
sqlite3VdbeJumpHere(v, addrL);
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index ca709b44f..afb60761f 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -780,7 +780,8 @@ pushOntoSorter(Parse * pParse, /* Parser context */
sqlite3VdbeAddOp2(v, OP_SorterInsert, pSort->iECursor,
regRecord);
} else {
- sqlite3VdbeAddOp2(v, OP_IdxInsert, regRecord, pSort->reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, regRecord, 0,
+ pSort->reg_eph);
}
if (iLimit) {
@@ -862,7 +863,7 @@ vdbe_insert_distinct(struct Parse *parse, int cursor, int reg_eph,
int r1 = sqlite3GetTempReg(parse);
sqlite3VdbeAddOp4Int(v, OP_Found, cursor, addr_repeat, reg_data, n);
sqlite3VdbeAddOp3(v, OP_MakeRecord, reg_data, n, r1);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, r1, reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, r1, 0, reg_eph);
sqlite3ReleaseTempReg(parse, r1);
}
@@ -1099,7 +1100,8 @@ selectInnerLoop(Parse * pParse, /* The parser context */
r1 = sqlite3GetTempReg(pParse);
sqlite3VdbeAddOp3(v, OP_MakeRecord, regResult,
nResultCol, r1);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, r1, pDest->reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, r1, 0,
+ pDest->reg_eph);
sqlite3ReleaseTempReg(pParse, r1);
break;
}
@@ -1142,7 +1144,7 @@ selectInnerLoop(Parse * pParse, /* The parser context */
sqlite3VdbeAddOp4Int(v, OP_Found, iParm + 1,
addr, r1, 0);
VdbeCoverage(v);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, r1,
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, r1, 0,
pDest->reg_eph + 1);
assert(pSort == 0);
}
@@ -1167,7 +1169,8 @@ selectInnerLoop(Parse * pParse, /* The parser context */
sqlite3VdbeAddOp3(v, OP_MakeRecord, regCopy, nResultCol + 1, regRec);
/* Set flag to save memory allocating one by malloc. */
sqlite3VdbeChangeP5(v, 1);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, regRec, pDest->reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, regRec, 0,
+ pDest->reg_eph);
sqlite3ReleaseTempReg(pParse, regRec);
sqlite3ReleaseTempRange(pParse, regCopy, nResultCol + 1);
}
@@ -1197,7 +1200,8 @@ selectInnerLoop(Parse * pParse, /* The parser context */
sqlite3ExprCacheAffinityChange(pParse,
regResult,
nResultCol);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, r1, pDest->reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, r1, 0,
+ pDest->reg_eph);
sqlite3ReleaseTempReg(pParse, r1);
}
break;
@@ -1280,7 +1284,7 @@ selectInnerLoop(Parse * pParse, /* The parser context */
sqlite3VdbeAddOp3(v, OP_MakeRecord, regResult,
nResultCol, r3);
if (eDest == SRT_DistQueue) {
- sqlite3VdbeAddOp2(v, OP_IdxInsert, r3,
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, r3, 0,
pDest->reg_eph + 1);
}
for (i = 0; i < nKey; i++) {
@@ -1291,7 +1295,8 @@ selectInnerLoop(Parse * pParse, /* The parser context */
}
sqlite3VdbeAddOp2(v, OP_Sequence, iParm, r2 + nKey);
sqlite3VdbeAddOp3(v, OP_MakeRecord, r2, nKey + 2, r1);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, r1, pDest->reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, r1, 0,
+ pDest->reg_eph);
if (addrTest)
sqlite3VdbeJumpHere(v, addrTest);
sqlite3ReleaseTempReg(pParse, r1);
@@ -1606,7 +1611,8 @@ generateSortTail(Parse * pParse, /* Parsing context */
regTupleid);
sqlite3VdbeAddOp3(v, OP_Copy, regRow, regCopy, nSortData - 1);
sqlite3VdbeAddOp3(v, OP_MakeRecord, regCopy, nColumn + 1, regRow);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, regRow, pDest->reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, regRow, 0,
+ pDest->reg_eph);
sqlite3ReleaseTempReg(pParse, regCopy);
break;
}
@@ -1616,7 +1622,8 @@ generateSortTail(Parse * pParse, /* Parsing context */
sqlite3VdbeAddOp4(v, OP_MakeRecord, regRow, nColumn,
regTupleid, pDest->zAffSdst, nColumn);
sqlite3ExprCacheAffinityChange(pParse, regRow, nColumn);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, regTupleid, pDest->reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, regTupleid, 0,
+ pDest->reg_eph);
break;
}
case SRT_Mem:{
@@ -3039,7 +3046,8 @@ generateOutputSubroutine(struct Parse *parse, struct Select *p,
in->nSdst + 1, regRec);
/* Set flag to save memory allocating one by malloc. */
sqlite3VdbeChangeP5(v, 1);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, regRec, dest->reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, regRec, 0,
+ dest->reg_eph);
sqlite3ReleaseTempRange(parse, regCopy, in->nSdst + 1);
sqlite3ReleaseTempReg(parse, regRec);
break;
@@ -3055,7 +3063,8 @@ generateOutputSubroutine(struct Parse *parse, struct Select *p,
in->nSdst);
sqlite3ExprCacheAffinityChange(parse, in->iSdst,
in->nSdst);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, r1, dest->reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, r1, 0,
+ dest->reg_eph);
sqlite3ReleaseTempReg(parse, r1);
break;
}
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 0e2d0fde8..cc69e2f30 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -284,7 +284,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
* malloc.
*/
sqlite3VdbeChangeP5(v, 1);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, regKey, reg_eph);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, regKey, 0, reg_eph);
}
/* End the database scan loop.
*/
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index bf6f4cdd1..8fdabc212 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4392,12 +4392,12 @@ case OP_SorterInsert: { /* in2 */
break;
}
-/* Opcode: IdxInsert P1 P2 * P4 P5
+/* Opcode: IdxInsert P1 * P3 P4 P5
* Synopsis: key=r[P1]
*
* @param P1 Index of a register with MessagePack data to insert.
- * @param P2 If P4 is not set, then P2 is register containing pointer
- * to space to insert into.
+ * @param P3 If P4 is not set, then P3 is register containing
+ * pointer to space to insert into.
* @param P4 Pointer to the struct space to insert to.
* @param P5 Flags. If P5 contains OPFLAG_NCHANGE, then VDBE
* accounts the change in a case of successful insertion in
@@ -4405,7 +4405,7 @@ case OP_SorterInsert: { /* in2 */
* we are processing INSERT OR INGORE statement. Thus, in
* case of conflict we don't raise an error.
*/
-/* Opcode: IdxReplace2 P1 * * P4 P5
+/* Opcode: IdxReplace2 P1 * P3 P4 P5
* Synopsis: key=r[P1]
*
* This opcode works exactly as IdxInsert does, but in Tarantool
@@ -4424,7 +4424,7 @@ case OP_IdxInsert: {
if (pOp->p4type == P4_SPACEPTR)
space = pOp->p4.space;
else
- space = aMem[pOp->p2].u.p;
+ space = aMem[pOp->p3].u.p;
assert(space != NULL);
if (space->def->id != 0) {
/* Make sure that memory has been allocated on region. */
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 9c3462bc0..9b0652357 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -878,7 +878,7 @@ constructAutomaticIndex(Parse * pParse, /* The parsing context */
regRecord = sqlite3GetTempReg(pParse);
regBase = sql_generate_index_key(pParse, pIdx, pLevel->iTabCur,
regRecord, NULL, 0);
- sqlite3VdbeAddOp2(v, OP_IdxInsert, pLevel->iIdxCur, regRecord);
+ sqlite3VdbeAddOp3(v, OP_IdxInsert, pLevel->iIdxCur, 0, regRecord);
if (pTabItem->fg.viaCoroutine) {
sqlite3VdbeChangeP2(v, addrCounter, regBase + n);
translateColumnToCopy(v, addrTop, pLevel->iTabCur,
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index b124a1d53..fee13cde3 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -1550,9 +1550,10 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about t
sqlite3VdbeAddOp3
(v, OP_MakeRecord,
r, pk_part_count, regPk);
- sqlite3VdbeAddOp2
+ sqlite3VdbeAddOp3
(v, OP_IdxInsert,
- regPk, reg_row_set);
+ regPk, 0,
+ reg_row_set);
}
/* Release the array of temp registers */
--
2.19.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [tarantool-patches] [PATCH v1 5/6] sql: fix incomplete UPDATE on IdxInsert failured
2018-11-29 13:09 [tarantool-patches] [PATCH v1 0/6] sql: Checks on server side Kirill Shcherbatov
` (3 preceding siblings ...)
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 4/6] sql: release OP_Idx{Insert,Replace} p2 argument Kirill Shcherbatov
@ 2018-11-29 13:09 ` Kirill Shcherbatov
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 6/6] sql: make sql checks on server side Kirill Shcherbatov
5 siblings, 0 replies; 9+ messages in thread
From: Kirill Shcherbatov @ 2018-11-29 13:09 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
Now the SQL UPDATE operation is a sequence of two operations -
removing the old data from the index and inserting new ones.
If, for some reason, the insertion of new data has not been done
for ON_REPLACE_IGNORE strategy, the old tuple is not restored.
We introduce a new argument p2 for OP_IdxReplace and OP_IdxInsert
opcodes containing label for VDBE error handler for the case of
insertion failure. Each update step makes a SAVEPOINT before
old tuple deletion and defines an error handler with ROLLBACK.
Need for #3691
---
| 2 ++
src/box/sql/insert.c | 12 ++++++++----
src/box/sql/sqliteInt.h | 7 ++++++-
src/box/sql/update.c | 31 ++++++++++++++++++++++++++++++-
src/box/sql/vdbe.c | 39 ++++++++++++++++++++++++++++++++++-----
5 files changed, 80 insertions(+), 11 deletions(-)
--git a/extra/mkopcodeh.sh b/extra/mkopcodeh.sh
index 5f31f2b7d..ea2d6b695 100755
--- a/extra/mkopcodeh.sh
+++ b/extra/mkopcodeh.sh
@@ -150,6 +150,8 @@ while [ "$i" -lt "$nOp" ]; do
eval "name=\$ARRAY_order_$i"
case "$name" in
# The following are the opcodes that are processed by resolveP2Values()
+ OP_IdxInsert | \
+ OP_IdxReplace | \
OP_Savepoint | \
OP_Checkpoint | \
OP_JournalMode | \
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index b099178d7..4fc39c233 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -763,7 +763,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
fkey_emit_check(pParse, pTab, 0, regIns, 0);
vdbe_emit_insertion_completion(v, space, regIns + 1,
pTab->def->field_count,
- on_error);
+ on_error, 0);
}
/* Update the count of rows that are inserted
@@ -1076,10 +1076,13 @@ process_index: ;
void
vdbe_emit_insertion_completion(struct Vdbe *v, struct space *space,
int raw_data_reg, uint32_t tuple_len,
- enum on_conflict_action on_conflict)
+ enum on_conflict_action on_conflict,
+ int conflict_handler_label)
{
assert(v != NULL);
u16 pik_flags = OPFLAG_NCHANGE;
+ if (conflict_handler_label != 0)
+ pik_flags |= OPFLAG_OE_HANDLER;
if (on_conflict == ON_CONFLICT_ACTION_IGNORE)
pik_flags |= OPFLAG_OE_IGNORE;
else if (on_conflict == ON_CONFLICT_ACTION_FAIL)
@@ -1088,8 +1091,9 @@ vdbe_emit_insertion_completion(struct Vdbe *v, struct space *space,
pik_flags |= OPFLAG_OE_ROLLBACK;
sqlite3VdbeAddOp3(v, OP_MakeRecord, raw_data_reg, tuple_len,
raw_data_reg + tuple_len);
- sqlite3VdbeAddOp1(v, OP_IdxInsert, raw_data_reg + tuple_len);
- sqlite3VdbeChangeP4(v, -1, (char *)space, P4_SPACEPTR);
+ sqlite3VdbeAddOp4(v, OP_IdxInsert, raw_data_reg + tuple_len,
+ conflict_handler_label, 0, (char *)space,
+ P4_SPACEPTR);
sqlite3VdbeChangeP5(v, pik_flags);
}
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index dbf58d967..9353798c8 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2829,6 +2829,8 @@ struct Parse {
#define OPFLAG_OE_IGNORE 0x200 /* OP_IdxInsert: Ignore flag */
#define OPFLAG_OE_FAIL 0x400 /* OP_IdxInsert: Fail flag */
#define OPFLAG_OE_ROLLBACK 0x800 /* OP_IdxInsert: Rollback flag. */
+/** OP_IdxInsert: Interrupt handler is set flag. */
+#define OPFLAG_OE_HANDLER 0x1000
#define OPFLAG_LENGTHARG 0x40 /* OP_Column only used for length() */
#define OPFLAG_TYPEOFARG 0x80 /* OP_Column only used for typeof() */
#define OPFLAG_SEEKEQ 0x02 /* OP_Open** cursor uses EQ seek only */
@@ -3869,11 +3871,14 @@ vdbe_emit_constraint_checks(struct Parse *parse_context,
* @param raw_data_reg Register with raw data to insert.
* @param tuple_len Number of registers to hold the tuple.
* @param on_conflict On conflict action.
+ * @param conflict_handler_label Label with error handler for
+ * on_conflict IGNORE strategy.
*/
void
vdbe_emit_insertion_completion(struct Vdbe *v, struct space *space,
int raw_data_reg, uint32_t tuple_len,
- enum on_conflict_action on_conflict);
+ enum on_conflict_action on_conflict,
+ int conflict_handler_label);
void
sql_set_multi_write(Parse *, bool);
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index cc69e2f30..6c47e5a80 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -34,6 +34,7 @@
* to handle UPDATE statements.
*/
#include "sqliteInt.h"
+#include "vdbeInt.h"
#include "box/session.h"
#include "tarantoolInt.h"
#include "box/schema.h"
@@ -94,6 +95,8 @@ sqlite3Update(Parse * pParse, /* The parser context */
int hasFK; /* True if foreign key processing is required */
int labelBreak; /* Jump here to break out of UPDATE loop */
int labelContinue; /* Jump here to continue next step of UPDATE loop */
+ /* Jump here to rollback record on Tarantool error. */
+ int rollback_change = 0;
struct session *user_session = current_session();
bool is_view; /* True when updating a view (INSTEAD OF trigger) */
@@ -112,6 +115,9 @@ sqlite3Update(Parse * pParse, /* The parser context */
int regNew = 0; /* Content of the NEW.* table in triggers */
int regOld = 0; /* Content of OLD.* table in triggers */
int regKey = 0; /* composite PRIMARY KEY value */
+ /* Token to make savepoints for UPDATE step. */
+ struct Token rollback_token =
+ {.z = "UPDATE_ITEM", .n = strlen("UPDATE_ITEM")};
db = pParse->db;
if (pParse->nErr || db->mallocFailed) {
@@ -440,13 +446,19 @@ sqlite3Update(Parse * pParse, /* The parser context */
int addr1 = sqlite3VdbeAddOp4Int(v, OP_NotFound, pk_cursor, 0,
regKey, nKey);
assert(regNew == regNewPk + 1);
+ /* Make savepoint for rollback OP_Delete */
+ if (!box_txn()) {
+ rollback_change = sqlite3VdbeMakeLabel(v);
+ sqlite3Savepoint(pParse, SAVEPOINT_BEGIN,
+ &rollback_token);
+ }
sqlite3VdbeAddOp2(v, OP_Delete, pk_cursor, 0);
sqlite3VdbeJumpHere(v, addr1);
if (hasFK)
fkey_emit_check(pParse, pTab, 0, regNewPk, aXRef);
vdbe_emit_insertion_completion(v, space, regNew,
pTab->def->field_count,
- on_error);
+ on_error, rollback_change);
/*
* Do any ON CASCADE, SET NULL or SET DEFAULT
* operations required to handle rows that refer
@@ -467,6 +479,23 @@ sqlite3Update(Parse * pParse, /* The parser context */
TRIGGER_AFTER, pTab, regOldPk, on_error,
labelContinue);
+ /* OR IGNORE/REPLACE/ABORT error handler. */
+ if (rollback_change != 0) {
+ sqlite3VdbeAddOp2(v, OP_Goto, 0, labelContinue);
+ sqlite3VdbeResolveLabel(v, rollback_change);
+ struct Vdbe *temp =
+ sqlite3DbMallocRaw(db, sizeof(struct Vdbe));
+ if (unlikely(temp == NULL))
+ goto update_cleanup;
+ sqlite3VdbeAddOp4(v, OP_ErrorCtx, 0, 0, 0,
+ (const char *)temp, P4_DYNAMIC);
+ sqlite3Savepoint(pParse, SAVEPOINT_ROLLBACK,
+ &rollback_token);
+ if (on_error != ON_CONFLICT_ACTION_IGNORE) {
+ sqlite3VdbeAddOp4(v, OP_ErrorCtx, 1, 0, 0,
+ (const char *)temp, P4_STATIC);
+ }
+ }
/* Repeat the above with the next record to be updated, until
* all record selected by the WHERE clause have been updated.
*/
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 8fdabc212..5efcbcf21 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -945,6 +945,27 @@ case OP_EndCoroutine: { /* in1 */
break;
}
+/* Opcode: ErrorCtx P1 * * P4 *
+ *
+ * Save VDBE error state in dummy uninitialized Vdbe object
+ * specified with P4. Use P1 = 0 for save and P1 = 1 for raise
+ * error context.
+ * Used to manage error outside of error handler.
+ */
+case OP_ErrorCtx: {
+ struct Vdbe *v = (struct Vdbe *)pOp->p4.p;
+ if (pOp->p1 == 0) {
+ v->rc = p->rc;
+ p->rc = SQLITE_OK;
+ v->errorAction = p->errorAction;
+ } else {
+ rc = v->rc;
+ pOp->p2 = v->errorAction;
+ goto abort_due_to_error;
+ }
+ break;
+}
+
/* Opcode: Yield P1 P2 * * *
*
* Swap the program counter with the value in register P1. This
@@ -4392,10 +4413,12 @@ case OP_SorterInsert: { /* in2 */
break;
}
-/* Opcode: IdxInsert P1 * P3 P4 P5
+/* Opcode: IdxInsert P1 P2 P3 P4 P5
* Synopsis: key=r[P1]
*
* @param P1 Index of a register with MessagePack data to insert.
+ * @param P2 The value to jump on error when OPFLAG_OE_HANDLER
+ * flag is set.
* @param P3 If P4 is not set, then P3 is register containing
* pointer to space to insert into.
* @param P4 Pointer to the struct space to insert to.
@@ -4405,14 +4428,14 @@ case OP_SorterInsert: { /* in2 */
* we are processing INSERT OR INGORE statement. Thus, in
* case of conflict we don't raise an error.
*/
-/* Opcode: IdxReplace2 P1 * P3 P4 P5
+/* Opcode: IdxReplace2 P1 P2 P3 P4 P5
* Synopsis: key=r[P1]
*
* This opcode works exactly as IdxInsert does, but in Tarantool
* internals it invokes box_replace() instead of box_insert().
*/
-case OP_IdxReplace:
-case OP_IdxInsert: {
+case OP_IdxReplace: /* jump */
+case OP_IdxInsert: { /* jump */
pIn2 = &aMem[pOp->p1];
assert((pIn2->flags & MEM_Blob) != 0);
if (pOp->p5 & OPFLAG_NCHANGE)
@@ -4443,7 +4466,8 @@ case OP_IdxInsert: {
if (pOp->p5 & OPFLAG_OE_IGNORE) {
/* Ignore any kind of failes and do not raise error message */
- rc = SQLITE_OK;
+ if ((pOp->p5 & OPFLAG_OE_HANDLER) == 0)
+ rc = SQLITE_OK;
/* If we are in trigger, increment ignore raised counter */
if (p->pFrame)
p->ignoreRaised++;
@@ -4452,6 +4476,11 @@ case OP_IdxInsert: {
} else if (pOp->p5 & OPFLAG_OE_ROLLBACK) {
p->errorAction = ON_CONFLICT_ACTION_ROLLBACK;
}
+ if (pOp->p5 & OPFLAG_OE_HANDLER && rc != SQLITE_OK) {
+ p->rc = rc;
+ rc = SQLITE_OK;
+ goto jump_to_p2;
+ }
if (rc != 0)
goto abort_due_to_error;
break;
--
2.19.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [tarantool-patches] [PATCH v1 6/6] sql: make sql checks on server side
2018-11-29 13:09 [tarantool-patches] [PATCH v1 0/6] sql: Checks on server side Kirill Shcherbatov
` (4 preceding siblings ...)
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 5/6] sql: fix incomplete UPDATE on IdxInsert failured Kirill Shcherbatov
@ 2018-11-29 13:09 ` Kirill Shcherbatov
2018-11-29 14:09 ` [tarantool-patches] " Kirill Shcherbatov
2018-11-29 18:41 ` Kirill Shcherbatov
5 siblings, 2 replies; 9+ messages in thread
From: Kirill Shcherbatov @ 2018-11-29 13:09 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
To make SQL CHECKS on space insertion in LUA we have introduced
a new sql_check class containing precompiled reusable VDBE making
required validations. It has been parameterized with binding
parameters to map a tuple to be inserted in VDBE memory and
to manage checks to be performed on UPDATE operation.
Closes #3691
---
src/box/alter.cc | 44 +++++++
src/box/space.c | 10 ++
src/box/space.h | 5 +
src/box/sql.c | 175 ++++++++++++++++++++++++++
src/box/sql.h | 66 ++++++++++
src/box/sql/build.c | 2 +-
src/box/sql/insert.c | 107 +++++-----------
src/box/sql/sqliteInt.h | 30 +++++
src/box/sql/vdbeInt.h | 7 ++
src/box/sql/vdbeapi.c | 8 --
test/sql-tap/check.test.lua | 24 ++--
test/sql-tap/fkey2.test.lua | 4 +-
test/sql-tap/table.test.lua | 8 +-
test/sql/checks.result | 51 ++++++++
test/sql/checks.test.lua | 18 +++
test/sql/gh-2981-check-autoinc.result | 8 +-
16 files changed, 457 insertions(+), 110 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 3e9b43a54..5b87074c3 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -39,6 +39,7 @@
#include "coll_id_def.h"
#include "txn.h"
#include "tuple.h"
+#include "trigger.h"
#include "fiber.h" /* for gc_pool */
#include "scoped_guard.h"
#include "third_party/base64.h"
@@ -1407,6 +1408,34 @@ on_drop_space_rollback(struct trigger *trigger, void *event)
space_cache_replace(NULL, space);
}
+/**
+ * SQL-specific actions for space.
+ */
+static void
+on_space_before_replace(struct trigger *trigger, void *event)
+{
+ uint32_t space_id = (uint32_t)(uintptr_t)trigger->data;
+ struct space *space = space_cache_find(space_id);
+ assert(space != NULL);
+ struct txn *txn = (struct txn *) event;
+ struct txn_stmt *stmt = txn_current_stmt(txn);
+ if (stmt == NULL)
+ return;
+ struct tuple *new_tuple = stmt->new_tuple;
+ struct tuple *old_tuple = stmt->old_tuple;
+ if (new_tuple != NULL && space->sql_checks != NULL) {
+ const char *old_tuple_raw = old_tuple != NULL ?
+ tuple_data(old_tuple) : NULL;
+ const char *new_tuple_raw = new_tuple != NULL ?
+ tuple_data(new_tuple) : NULL;
+ if (sql_checks_run(space->sql_checks,
+ space->def->opts.checks_ast, space->def,
+ old_tuple_raw, new_tuple_raw) != 0) {
+ diag_raise();
+ }
+ }
+}
+
/**
* Run the triggers registered on commit of a change in _space.
*/
@@ -1725,6 +1754,21 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
txn_on_rollback(txn, on_rollback_view);
select_guard.is_active = false;
}
+ /* Setup SQL-actions trigger if required. */
+ if (def->opts.checks_ast != NULL) {
+ struct trigger *sql_actions_trigger =
+ (struct trigger *)malloc(sizeof(struct trigger));
+ if (sql_actions_trigger == NULL) {
+ tnt_raise(OutOfMemory, sizeof(struct trigger),
+ "calloc",
+ "on_space_before_replace");
+ }
+ trigger_create(sql_actions_trigger,
+ on_space_before_replace,
+ (void *)(uintptr_t)space->def->id,
+ (trigger_f0)free);
+ trigger_add(&space->before_replace, sql_actions_trigger);
+ }
} else if (new_tuple == NULL) { /* DELETE */
access_check_ddl(old_space->def->name, old_space->def->id,
old_space->def->uid, SC_SPACE, PRIV_D);
diff --git a/src/box/space.c b/src/box/space.c
index 4d174f7ec..6da0e6e9a 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -29,6 +29,7 @@
* SUCH DAMAGE.
*/
#include "space.h"
+#include "sql.h"
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
@@ -165,6 +166,13 @@ space_create(struct space *space, struct engine *engine,
space_fill_index_map(space);
rlist_create(&space->parent_fkey);
rlist_create(&space->child_fkey);
+ if (def->opts.checks_ast != NULL) {
+ struct sql_checks *checks =
+ sql_checks_create(sql_get(), def->opts.checks_ast, def);
+ if (unlikely(checks == NULL))
+ goto fail_free_indexes;
+ space->sql_checks = checks;
+ }
return 0;
fail_free_indexes:
@@ -216,6 +224,8 @@ space_delete(struct space *space)
trigger_destroy(&space->before_replace);
trigger_destroy(&space->on_replace);
trigger_destroy(&space->on_stmt_begin);
+ if (space->sql_checks != NULL)
+ sql_checks_destroy(space->sql_checks);
space_def_delete(space->def);
/*
* SQL Triggers should be deleted with
diff --git a/src/box/space.h b/src/box/space.h
index 7eb7ae292..e83609992 100644
--- a/src/box/space.h
+++ b/src/box/space.h
@@ -150,6 +150,11 @@ struct space {
struct rlist on_stmt_begin;
/** SQL Trigger list. */
struct sql_trigger *sql_triggers;
+ /**
+ * The SQL Checks to perform on INSERT, REPLACE and
+ * UPDATE operations.
+ */
+ struct sql_checks *sql_checks;
/**
* The number of *enabled* indexes in the space.
*
diff --git a/src/box/sql.c b/src/box/sql.c
index 2b710fab6..b9d67ef07 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -34,6 +34,7 @@
#include "sql/sqliteInt.h"
#include "sql/tarantoolInt.h"
#include "sql/vdbeInt.h"
+#include "execute.h"
#include "index.h"
#include "info.h"
@@ -1538,3 +1539,177 @@ sql_checks_resolve_space_def_reference(ExprList *expr_list,
sql_parser_destroy(&parser);
return rc;
}
+
+static int
+check_is_enabled_callback(struct Walker *w, Expr *expr)
+{
+ /* CHECK constraint uses a changing column. */
+ const int marker = 0x01;
+ if (expr->op == TK_COLUMN) {
+ assert(expr->iColumn >= 0 || expr->iColumn == -1);
+ if (expr->iColumn >= 0) {
+ if (w->u.aiCol[expr->iColumn] >= 0)
+ w->eCode |= marker;
+ }
+ }
+ return WRC_Continue;
+}
+
+/**
+ * Return true if this CHECK constraint can't be skipped when
+ * validating the new row in the UPDATE statement. In other
+ * words return false if CHECK constraint check does not use any of the
+ * changing columns.
+ * @param check CHECK constraint on a row that is being UPDATE-ed.
+ * The only columns that are modified by the UPDATE
+ * are those for which update_mask[i] >= 0.
+ * @param update_mask Array of items that were modified.
+ * 0 for unchanged field, -1 for modified.
+ * @retval 1 when specified check is required, 0 elsewhere.
+ */
+static bool
+sql_check_is_enabled(struct Expr *check, int *update_mask)
+{
+ struct Walker w;
+ memset(&w, 0, sizeof(w));
+ w.eCode = 0;
+ w.xExprCallback = check_is_enabled_callback;
+ w.u.aiCol = update_mask;
+ sqlite3WalkExpr(&w, check);
+ return w.eCode == 0 ? 1 : 0;
+}
+
+struct sql_checks *
+sql_checks_create(struct sqlite3 *db, struct ExprList *checks_ast,
+ struct space_def *def)
+{
+ assert(checks_ast != NULL);
+ struct sql_checks *checks = malloc(sizeof(struct sql_checks));
+ if (unlikely(checks == NULL)) {
+ diag_set(OutOfMemory, sizeof(struct sql_checks), "malloc",
+ "checks");
+ return NULL;
+ }
+
+ struct Parse parser;
+ sql_parser_create(&parser, db);
+ struct Vdbe *v = sqlite3GetVdbe(&parser);
+ if (unlikely(v == NULL)) {
+ diag_set(ClientError, ER_SQL_EXECUTE, sqlite3_errmsg(db));
+ free(checks);
+ return NULL;
+ }
+ /* Compile VDBE with default sql parameters. */
+ struct session *user_session = current_session();
+ uint32_t sql_flags = user_session->sql_flags;
+ user_session->sql_flags = default_sql_flags;
+
+ /* Make continuous registers allocation. */
+ uint32_t reg_count = def->field_count + checks_ast->nExpr;
+ int new_tuple_reg = sqlite3GetTempRange(&parser, reg_count);
+ int checks_state_reg = def->field_count + 1;
+ /*
+ * Generate a prologe code to bind variables new_tuple_var
+ * and checks_state_var to new_tuple_reg and
+ * checks_state_reg respectively.
+ */
+ struct Expr bind = {.op = TK_VARIABLE, .u.zToken = "?"};
+ checks->new_tuple_var = parser.nVar + 1;
+ checks->checks_state_var = checks->new_tuple_var + def->field_count;
+ for (uint32_t i = 0; i < reg_count; i++) {
+ sqlite3ExprAssignVarNumber(&parser, &bind, 1);
+ sqlite3ExprCodeTarget(&parser, &bind, new_tuple_reg + i);
+ }
+ /* Generate Checks code. */
+ vdbe_emit_checks_test(&parser, checks_ast, def, new_tuple_reg,
+ checks_state_reg);
+ sql_finish_coding(&parser);
+ sql_parser_destroy(&parser);
+
+ user_session->sql_flags = sql_flags;
+ checks->stmt = (struct sqlite3_stmt *)v;
+ return checks;
+}
+
+void
+sql_checks_destroy(struct sql_checks *checks)
+{
+ sqlite3_finalize(checks->stmt);
+ free(checks);
+}
+
+int
+sql_checks_run(const struct sql_checks *checks,
+ const struct ExprList *checks_ast, struct space_def *def,
+ const char *old_tuple_raw, const char *new_tuple_raw)
+{
+ assert(new_tuple_raw != NULL);
+ uint32_t field_count = def->field_count;
+ struct sqlite3_stmt *stmt = checks->stmt;
+ struct region *region = &fiber()->gc;
+ int *update_mask = NULL;
+ if (old_tuple_raw != NULL) {
+ /* The update_mask is required only for UPDATE. */
+ uint32_t update_mask_sz = sizeof(int) * field_count;
+ update_mask = region_alloc(region, update_mask_sz);
+ if (unlikely(update_mask == NULL)) {
+ diag_set(OutOfMemory, update_mask_sz, "region_alloc",
+ "update_mask");
+ return -1;
+ }
+ }
+ /*
+ * Prepare parameters for checks->stmt execution:
+ * - Unpacked new tuple fields mapped to Vdbe memory from
+ * variables from range:
+ * [new_tuple_var,new_tuple_var+field_count]
+ * - Sequence of Checks status parameters(is Check with
+ * index i enabled) mapped to Vdbe memory
+ * [checks_state_var,checks_state_var+checks_ast->nExpr]
+ * Items are calculated with sql_check_is_enabled
+ * routine called with update_mask for UPDATE operation.
+ * In case of regular INSERT, REPLACE operation all
+ * Checks should be called so all checks_state_reg items
+ * are 1.
+ */
+ mp_decode_array(&new_tuple_raw);
+ if (old_tuple_raw != NULL)
+ mp_decode_array(&old_tuple_raw);
+ /* Reset VDBE to make new bindings. */
+ sqlite3_reset(stmt);
+ for (uint32_t i = 0; i < field_count; i++) {
+ const char *new_tuple_field = new_tuple_raw;
+ struct sql_bind bind;
+ if (sql_bind_decode(&bind, checks->new_tuple_var + i,
+ &new_tuple_raw) != 0)
+ return -1;
+ if (sql_bind_column(stmt, &bind,
+ checks->new_tuple_var + i) != 0)
+ return -1;
+ /* Skip calculations in case of INSERT. */
+ if (update_mask == NULL)
+ continue;
+ /* Form the tuple update mask. */
+ const char *old_tuple_field = old_tuple_raw;
+ mp_next(&old_tuple_raw);
+ update_mask[i] = (old_tuple_raw - old_tuple_field !=
+ new_tuple_raw - new_tuple_field ||
+ memcmp(new_tuple_field, old_tuple_field,
+ old_tuple_raw -
+ old_tuple_field) != 0) ? -1 : 0;
+ }
+ /* Map Checks "is enabled" paremeters. */
+ for (int i = 0; i < checks_ast->nExpr; i++) {
+ struct Expr *expr = checks_ast->a[i].pExpr;
+ int val = update_mask != NULL ?
+ sql_check_is_enabled(expr, update_mask) : 1;
+ sqlite3_bind_int64(stmt, checks->checks_state_var + i, val);
+ }
+ /* Checks VDBE can't expire, reset expired flag & Burn. */
+ struct Vdbe *v = (struct Vdbe *)stmt;
+ v->expired = 0;
+ int rc = sqlite3_step(stmt) == SQLITE_DONE ? 0 : -1;
+ if (rc != 0)
+ diag_set(ClientError, ER_SQL_EXECUTE, v->zErrMsg);
+ return rc;
+}
diff --git a/src/box/sql.h b/src/box/sql.h
index ec6d9d3cc..b4a96005f 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -68,6 +68,29 @@ struct Select;
struct Table;
struct sql_trigger;
+/** SQL Checks object. */
+struct sql_checks {
+ /**
+ * The first in an array variables representing the
+ * new tuple to be inserted.
+ */
+ int new_tuple_var;
+ /**
+ * The first in array of variables representing the state
+ * (1 for ON and 0 for OFF) of corresponding Check. This
+ * is the UPDATE optimization - disabled checks would be
+ * skipped.
+ */
+ int checks_state_var;
+ /**
+ * Precompiled reusable VDBE program for SQL Checks
+ * raising error on Check constraint failure. VDBE
+ * program assumes new_tuple and checks_state actual
+ * parameters to be placed in VDBE memory before run.
+ */
+ struct sqlite3_stmt *stmt;
+};
+
/**
* Perform parsing of provided expression. This is done by
* surrounding the expression w/ 'SELECT ' prefix and perform
@@ -404,6 +427,49 @@ sql_select_from_table_count(const struct Select *select);
const char *
sql_select_from_table_name(const struct Select *select, int i);
+/**
+ * Create sql_checks object by Checks AST for specified space.
+ * @param db Database handler.
+ * @param checks_ast Checks AST expression list. Checks space def
+ * references may be modified during
+ * construction.
+ * @param def Space definition that checks_ast should refer to.
+ * @retval NULL on error.
+ * @retval not NULL SQL Checks object pointer on success.
+*/
+struct sql_checks *
+sql_checks_create(struct sqlite3 *db, struct ExprList *checks_ast,
+ struct space_def *def);
+
+/**
+ * Destroy SQL Checks object, release related resources.
+ * @param checks SQL Checks object to destroy.
+ */
+void
+sql_checks_destroy(struct sql_checks *checks);
+
+/**
+ * Run SQL Checks VDBE with parameters constructed by Checks AST,
+ * Space definition and alter tuples. Checks AST and Space
+ * definition must match be the same that were used to create the
+ * SQL Checks object.
+ * @param checks SQL Checks object containing VDBE to execute.
+ * @param checks_ast Checks AST object to determine Checks that
+ * may be omitted on execution (to prepare
+ * checks_state parameter).
+ * @param def Space definition describing alter tuples format.
+ * @param old_tuple_raw Old tuple to be replaced in UPDATE operation.
+ * (NULL for INSERT)
+ * @param new_tuple_raw New tuple to be inserted.
+ * @retval 0 on all required Checks were passed.
+ * @retval -1 on system error or Check constrints failure
+ * indicating an diag_msg to be raised.
+ */
+int
+sql_checks_run(const struct sql_checks *checks,
+ const struct ExprList *checks_ast, struct space_def *def,
+ const char *old_tuple_raw, const char *new_tuple_raw);
+
#if defined(__cplusplus)
} /* extern "C" { */
#endif
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 4598087ae..6e7e43434 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -968,7 +968,7 @@ struct ExprList *
space_checks_expr_list(uint32_t space_id)
{
struct space *space;
- space = space_by_id(space_id);
+ space = space_cache_find(space_id);
assert(space != NULL);
assert(space->def != NULL);
return space->def->opts.checks_ast;
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 4fc39c233..0fb0a437d 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -813,51 +813,36 @@ sqlite3Insert(Parse * pParse, /* Parser context */
sqlite3DbFree(db, aRegIdx);
}
-/*
- * Meanings of bits in of pWalker->eCode for checkConstraintUnchanged()
- */
-#define CKCNSTRNT_COLUMN 0x01 /* CHECK constraint uses a changing column */
-
-/* This is the Walker callback from checkConstraintUnchanged(). Set
- * bit 0x01 of pWalker->eCode if
- * pWalker->eCode to 0 if this expression node references any of the
- * columns that are being modifed by an UPDATE statement.
- */
-static int
-checkConstraintExprNode(Walker * pWalker, Expr * pExpr)
+void
+vdbe_emit_checks_test(struct Parse *parser, struct ExprList *checks_ast,
+ struct space_def *def, int new_tuple_reg,
+ int checks_state_reg)
{
- if (pExpr->op == TK_COLUMN) {
- assert(pExpr->iColumn >= 0 || pExpr->iColumn == -1);
- if (pExpr->iColumn >= 0) {
- if (pWalker->u.aiCol[pExpr->iColumn] >= 0) {
- pWalker->eCode |= CKCNSTRNT_COLUMN;
- }
- }
+ struct Vdbe *v = sqlite3GetVdbe(parser);
+ assert(checks_ast != NULL);
+ int check_state_off = sqlite3GetTempReg(parser);
+ sqlite3VdbeAddOp2(v, OP_Integer, 0, check_state_off);
+ /*
+ * For CHECK constraint and for INSERT/UPDATE conflict
+ * action DEFAULT and ABORT in fact has the same meaning.
+ */
+ parser->ckBase = new_tuple_reg;
+ for (int i = 0; i < checks_ast->nExpr; i++) {
+ struct Expr *expr = checks_ast->a[i].pExpr;
+ int all_ok = sqlite3VdbeMakeLabel(v);
+ /* Skip check when it turned off. */
+ sqlite3VdbeAddOp3(v, OP_Eq, checks_state_reg + i, all_ok,
+ check_state_off);
+ sqlite3ExprIfTrue(parser, expr, all_ok, SQLITE_JUMPIFNULL);
+ char *name = checks_ast->a[i].zName;
+ if (name == NULL)
+ name = def->name;
+ sqlite3HaltConstraint(parser, SQLITE_CONSTRAINT_CHECK,
+ ON_CONFLICT_ACTION_ABORT, name,
+ P4_TRANSIENT, P5_ConstraintCheck);
+ sqlite3VdbeResolveLabel(v, all_ok);
}
- return WRC_Continue;
-}
-
-/*
- * pExpr is a CHECK constraint on a row that is being UPDATE-ed. The
- * only columns that are modified by the UPDATE are those for which
- * aiChng[i]>=0.
- *
- * Return true if CHECK constraint pExpr does not use any of the
- * changing columns. In other words, return true if this CHECK constraint
- * can be skipped when validating the new row in the UPDATE statement.
- */
-static int
-checkConstraintUnchanged(Expr * pExpr, int *aiChng)
-{
- Walker w;
- memset(&w, 0, sizeof(w));
- w.eCode = 0;
- w.xExprCallback = checkConstraintExprNode;
- w.u.aiCol = aiChng;
- sqlite3WalkExpr(&w, pExpr);
- testcase(w.eCode == 0);
- testcase(w.eCode == CKCNSTRNT_COLUMN);
- return !w.eCode;
+ sqlite3ReleaseTempReg(parser, check_state_off);
}
void
@@ -928,42 +913,6 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct Table *tab,
unreachable();
}
}
- /*
- * For CHECK constraint and for INSERT/UPDATE conflict
- * action DEFAULT and ABORT in fact has the same meaning.
- */
- if (on_conflict == ON_CONFLICT_ACTION_DEFAULT)
- on_conflict = ON_CONFLICT_ACTION_ABORT;
- /* Test all CHECK constraints. */
- struct ExprList *checks = space_checks_expr_list(def->id);
- enum on_conflict_action on_conflict_check = on_conflict;
- if (on_conflict == ON_CONFLICT_ACTION_REPLACE)
- on_conflict_check = ON_CONFLICT_ACTION_ABORT;
- if (checks != NULL) {
- parse_context->ckBase = new_tuple_reg;
- for (int i = 0; i < checks->nExpr; i++) {
- struct Expr *expr = checks->a[i].pExpr;
- if (is_update &&
- checkConstraintUnchanged(expr, upd_cols))
- continue;
- int all_ok = sqlite3VdbeMakeLabel(v);
- sqlite3ExprIfTrue(parse_context, expr, all_ok,
- SQLITE_JUMPIFNULL);
- if (on_conflict == ON_CONFLICT_ACTION_IGNORE) {
- sqlite3VdbeGoto(v, ignore_label);
- } else {
- char *name = checks->a[i].zName;
- if (name == NULL)
- name = def->name;
- sqlite3HaltConstraint(parse_context,
- SQLITE_CONSTRAINT_CHECK,
- on_conflict_check, name,
- P4_TRANSIENT,
- P5_ConstraintCheck);
- }
- sqlite3VdbeResolveLabel(v, all_ok);
- }
- }
sql_emit_table_affinity(v, tab->def, new_tuple_reg);
/*
* If PK is marked as INTEGER, use it as strict type,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 9353798c8..bbf53cbe3 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -594,6 +594,17 @@ sqlite3_column_value(sqlite3_stmt *,
int
sqlite3_finalize(sqlite3_stmt * pStmt);
+/*
+ * Terminate the current execution of an SQL statement and reset
+ * it back to its starting state so that it can be reused. A
+ * success code from the prior execution is returned.
+ *
+ * This routine sets the error code and string returned by
+ * sqlite3_errcode(), sqlite3_errmsg() and sqlite3_errmsg16().
+ */
+int
+sqlite3_reset(sqlite3_stmt * pStmt);
+
int
sqlite3_exec(sqlite3 *, /* An open database */
const char *sql, /* SQL to be evaluated */
@@ -3857,6 +3868,25 @@ vdbe_emit_constraint_checks(struct Parse *parse_context,
struct Table *tab, int new_tuple_reg,
enum on_conflict_action on_conflict,
int ignore_label, int *upd_cols);
+/**
+ * This routine generates code to make Check constraints
+ * validations on tuple insertion during INSERT, REPLACE or
+ * UPDATE operation.
+ *
+ * @param parser Current parsing context.
+ * @param checks_ast Checks AST Expression list.
+ * @param def Destination space definition.
+ * @param new_tuple_reg The first register of group containing
+ * fields of tuple to be inserted.
+ * @param checks_state_reg The first register of group contining
+ * enabled state of corresponding checks:
+ * 0 for disabled check to be skipped,
+ * 1 - for enabled.
+ */
+void
+vdbe_emit_checks_test(struct Parse *parser, struct ExprList *checks_ast,
+ struct space_def *def, int new_tuple_reg,
+ int checks_state_reg);
/**
* This routine generates code to finish the INSERT or UPDATE
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 50bc35b2b..ff341d7ae 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -552,4 +552,11 @@ int sqlite3VdbeRecordCompareMsgpack(const void *key1,
struct UnpackedRecord *key2);
u32 sqlite3VdbeMsgpackGet(const unsigned char *buf, Mem * pMem);
+static inline char *
+sql_stmt_err(struct sqlite3_stmt *stmt)
+{
+ struct Vdbe *v = (struct Vdbe *)stmt;
+ return v->zErrMsg;
+}
+
#endif /* !defined(SQLITE_VDBEINT_H) */
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 9e57af051..c1b0af2bc 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -132,14 +132,6 @@ sqlite3_finalize(sqlite3_stmt * pStmt)
return rc;
}
-/*
- * Terminate the current execution of an SQL statement and reset it
- * back to its starting state so that it can be reused. A success code from
- * the prior execution is returned.
- *
- * This routine sets the error code and string returned by
- * sqlite3_errcode(), sqlite3_errmsg() and sqlite3_errmsg16().
- */
int
sqlite3_reset(sqlite3_stmt * pStmt)
{
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index e78f06b3b..bfd126ddc 100755
--- a/test/sql-tap/check.test.lua
+++ b/test/sql-tap/check.test.lua
@@ -55,7 +55,7 @@ test:do_catchsql_test(
INSERT INTO t1 VALUES(6,7, 2);
]], {
-- <check-1.3>
- 1, "CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
-- </check-1.3>
})
@@ -75,7 +75,7 @@ test:do_catchsql_test(
INSERT INTO t1 VALUES(4,3, 2);
]], {
-- <check-1.5>
- 1, "CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
-- </check-1.5>
})
@@ -147,7 +147,7 @@ test:do_catchsql_test(
UPDATE t1 SET x=7 WHERE x==2
]], {
-- <check-1.12>
- 1, "CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
-- </check-1.12>
})
@@ -167,7 +167,7 @@ test:do_catchsql_test(
UPDATE t1 SET x=5 WHERE x==2
]], {
-- <check-1.14>
- 1, "CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
-- </check-1.14>
})
@@ -330,7 +330,7 @@ test:do_catchsql_test(
INSERT INTO t3 VALUES(111,222,333);
]], {
-- <check-3.9>
- 1, "CHECK constraint failed: T3"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T3"
-- </check-3.9>
})
@@ -401,7 +401,7 @@ test:do_catchsql_test(
UPDATE t4 SET x=0, y=1;
]], {
-- <check-4.6>
- 1, "CHECK constraint failed: T4"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T4"
-- </check-4.6>
})
@@ -421,7 +421,7 @@ test:do_catchsql_test(
UPDATE t4 SET x=0, y=2;
]], {
-- <check-4.9>
- 1, "CHECK constraint failed: T4"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T4"
-- </check-4.9>
})
@@ -498,7 +498,7 @@ test:do_catchsql_test(
UPDATE OR FAIL t1 SET x=7-x, y=y+1;
]], {
-- <check-6.5>
- 1, "CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
-- </check-6.5>
})
@@ -520,7 +520,7 @@ test:do_catchsql_test(
INSERT OR ROLLBACK INTO t1 VALUES(8,40.0, 10);
]], {
-- <check-6.7>
- 1, "CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
-- </check-6.7>
})
@@ -553,7 +553,7 @@ test:do_catchsql_test(
REPLACE INTO t1 VALUES(6,7, 11);
]], {
-- <check-6.12>
- 1, "CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
-- </check-6.12>
})
@@ -617,7 +617,7 @@ test:do_catchsql_test(
7.3,
" INSERT INTO t6 VALUES(11) ", {
-- <7.3>
- 1, "CHECK constraint failed: T6"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T6"
-- </7.3>
})
@@ -672,7 +672,7 @@ test:do_test(
return test:catchsql(" INSERT INTO t6 VALUES(12) ", "db2")
end, {
-- <7.8>
- 1, "CHECK constraint failed: T6"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T6"
-- </7.8>
})
end
diff --git a/test/sql-tap/fkey2.test.lua b/test/sql-tap/fkey2.test.lua
index 61db29f32..8c6072f75 100755
--- a/test/sql-tap/fkey2.test.lua
+++ b/test/sql-tap/fkey2.test.lua
@@ -361,7 +361,7 @@ test:do_catchsql_test(
UPDATE ab SET a = 5;
]], {
-- <fkey2-3.2>
- 1, "CHECK constraint failed: EF"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: EF"
-- </fkey2-3.2>
})
@@ -381,7 +381,7 @@ test:do_catchsql_test(
UPDATE ab SET a = 5;
]], {
-- <fkey2-3.4>
- 1, "CHECK constraint failed: EF"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: EF"
-- </fkey2-3.4>
})
diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
index 7fd9bac9f..4ed078e7a 100755
--- a/test/sql-tap/table.test.lua
+++ b/test/sql-tap/table.test.lua
@@ -1217,7 +1217,7 @@ test:do_catchsql_test(
INSERT INTO T21 VALUES(1, -1, 1);
]], {
-- <table-21.3>
- 1, "CHECK constraint failed: T21"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T21"
-- </table-21.3>
})
@@ -1227,7 +1227,7 @@ test:do_catchsql_test(
INSERT INTO T21 VALUES(1, 1, -1);
]], {
-- <table-21.4>
- 1, "CHECK constraint failed: T21"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: T21"
-- </table-21.4>
})
@@ -1368,7 +1368,7 @@ test:do_catchsql_test(
INSERT INTO T28 VALUES(0);
]], {
-- <table-22.10>
- 1, "CHECK constraint failed: CHECK1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CHECK1"
-- </table-22.10>
})
@@ -1378,7 +1378,7 @@ test:do_catchsql_test(
INSERT INTO T28 VALUES(9);
]], {
-- <table-22.11>
- 1, "CHECK constraint failed: CHECK2"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CHECK2"
-- </table-22.11>
})
diff --git a/test/sql/checks.result b/test/sql/checks.result
index 12a3aa14c..2078ef111 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -139,6 +139,57 @@ s = box.space._space:insert(t)
- error: 'Wrong space options (field 5): invalid expression specified (SQL error:
bindings are not allowed in DDL)'
...
+--
+-- gh-3691: Do SQL checks on server side
+--
+box.sql.execute("CREATE TABLE t1(x INTEGER CONSTRAINT ONE CHECK( x<5 ), y REAL CONSTRAINT TWO CHECK( y>x ), z INTEGER PRIMARY KEY);")
+---
+...
+box.space.T1:insert({7, 1, 1})
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: ONE'
+...
+box.space.T1:insert({2, 1, 1})
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: TWO'
+...
+box.space.T1:insert({2, 4, 1})
+---
+- [2, 4, 1]
+...
+box.space.T1:update({1}, {{"=", 1, 7}})
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: ONE'
+...
+box.sql.execute("DROP TABLE t1");
+---
+...
+opts = {checks = {{expr ='X > 5', name = 'ONE'}}}
+---
+...
+format = {{name = 'X', type = 'unsigned'}}
+---
+...
+t = {513, 1, 'test', 'memtx', 0, opts, format}
+---
+...
+s = box.space._space:insert(t)
+---
+...
+_ = box.space.test:create_index('pk')
+---
+...
+box.space.test:insert({1})
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: ONE'
+...
+box.space.test:insert({6})
+---
+- [6]
+...
+box.space.test:drop()
+---
+...
test_run:cmd("clear filter")
---
- true
diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua
index 0582bbb63..5fb6f65dc 100644
--- a/test/sql/checks.test.lua
+++ b/test/sql/checks.test.lua
@@ -60,5 +60,23 @@ format = {{name = 'X', type = 'unsigned'}}
t = {513, 1, 'test', 'memtx', 0, opts, format}
s = box.space._space:insert(t)
+--
+-- gh-3691: Do SQL checks on server side
+--
+box.sql.execute("CREATE TABLE t1(x INTEGER CONSTRAINT ONE CHECK( x<5 ), y REAL CONSTRAINT TWO CHECK( y>x ), z INTEGER PRIMARY KEY);")
+box.space.T1:insert({7, 1, 1})
+box.space.T1:insert({2, 1, 1})
+box.space.T1:insert({2, 4, 1})
+box.space.T1:update({1}, {{"=", 1, 7}})
+box.sql.execute("DROP TABLE t1");
+
+opts = {checks = {{expr ='X > 5', name = 'ONE'}}}
+format = {{name = 'X', type = 'unsigned'}}
+t = {513, 1, 'test', 'memtx', 0, opts, format}
+s = box.space._space:insert(t)
+_ = box.space.test:create_index('pk')
+box.space.test:insert({1})
+box.space.test:insert({6})
+box.space.test:drop()
test_run:cmd("clear filter")
diff --git a/test/sql/gh-2981-check-autoinc.result b/test/sql/gh-2981-check-autoinc.result
index b0f55e61d..06b79e14f 100644
--- a/test/sql/gh-2981-check-autoinc.result
+++ b/test/sql/gh-2981-check-autoinc.result
@@ -24,28 +24,28 @@ box.sql.execute("insert into t1 values (18, null);")
...
box.sql.execute("insert into t1(s2) values (null);")
---
-- error: 'CHECK constraint failed: T1'
+- error: 'Failed to execute SQL statement: CHECK constraint failed: T1'
...
box.sql.execute("insert into t2 values (18, null);")
---
...
box.sql.execute("insert into t2(s2) values (null);")
---
-- error: 'CHECK constraint failed: T2'
+- error: 'Failed to execute SQL statement: CHECK constraint failed: T2'
...
box.sql.execute("insert into t2 values (24, null);")
---
...
box.sql.execute("insert into t2(s2) values (null);")
---
-- error: 'CHECK constraint failed: T2'
+- error: 'Failed to execute SQL statement: CHECK constraint failed: T2'
...
box.sql.execute("insert into t3 values (9, null)")
---
...
box.sql.execute("insert into t3(s2) values (null)")
---
-- error: 'CHECK constraint failed: T3'
+- error: 'Failed to execute SQL statement: CHECK constraint failed: T3'
...
box.sql.execute("DROP TABLE t1")
---
--
2.19.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [tarantool-patches] Re: [PATCH v1 6/6] sql: make sql checks on server side
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 6/6] sql: make sql checks on server side Kirill Shcherbatov
@ 2018-11-29 14:09 ` Kirill Shcherbatov
2018-11-29 18:41 ` Kirill Shcherbatov
1 sibling, 0 replies; 9+ messages in thread
From: Kirill Shcherbatov @ 2018-11-29 14:09 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
Sorry, there are a few minor fixes that I pushed on branch.
> + if (new_tuple != NULL && space->sql_checks != NULL) {
> + const char *old_tuple_raw = old_tuple != NULL ?
> + tuple_data(old_tuple) : NULL;
> + const char *new_tuple_raw = new_tuple != NULL ?
> + tuple_data(new_tuple) : NULL;
I've dropped this redundant new_tuple != NULL on branch
> +/*
> + * Terminate the current execution of an SQL statement and reset
> + * it back to its starting state so that it can be reused. A
> + * success code from the prior execution is returned.
> + *
> + * This routine sets the error code and string returned by
> + * sqlite3_errcode(), sqlite3_errmsg() and sqlite3_errmsg16().
> + */
> +int
> +sqlite3_reset(sqlite3_stmt * pStmt);
I've written doxigen comment on branch and have refactored body to follow
our codestyle.
> +static inline char *
> +sql_stmt_err(struct sqlite3_stmt *stmt)
> +{
> + struct Vdbe *v = (struct Vdbe *)stmt;
> + return v->zErrMsg;
> +}
I've dropped this useless routine on branch.
^ permalink raw reply [flat|nested] 9+ messages in thread* [tarantool-patches] Re: [PATCH v1 6/6] sql: make sql checks on server side
2018-11-29 13:09 ` [tarantool-patches] [PATCH v1 6/6] sql: make sql checks on server side Kirill Shcherbatov
2018-11-29 14:09 ` [tarantool-patches] " Kirill Shcherbatov
@ 2018-11-29 18:41 ` Kirill Shcherbatov
1 sibling, 0 replies; 9+ messages in thread
From: Kirill Shcherbatov @ 2018-11-29 18:41 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
Working with Functional Indexes patch I've found this problem here.
Believe that space trigger should not be a part of DDL logic
diff --git a/src/box/alter.cc b/src/box/alter.cc
index aa75457ac..8226d7624 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1408,33 +1408,6 @@ on_drop_space_rollback(struct trigger *trigger, void *event)
space_cache_replace(NULL, space);
}
-/**
- * SQL-specific actions for space.
- */
-static void
-on_space_before_replace(struct trigger *trigger, void *event)
-{
- uint32_t space_id = (uint32_t)(uintptr_t)trigger->data;
- struct space *space = space_cache_find(space_id);
- assert(space != NULL);
- struct txn *txn = (struct txn *) event;
- struct txn_stmt *stmt = txn_current_stmt(txn);
- if (stmt == NULL)
- return;
- struct tuple *new_tuple = stmt->new_tuple;
- struct tuple *old_tuple = stmt->old_tuple;
- if (new_tuple != NULL && space->sql_checks != NULL) {
- const char *new_tuple_raw = tuple_data(new_tuple);
- const char *old_tuple_raw = old_tuple != NULL ?
- tuple_data(old_tuple) : NULL;
- if (sql_checks_run(space->sql_checks,
- space->def->opts.checks_ast, space->def,
- old_tuple_raw, new_tuple_raw) != 0) {
- diag_raise();
- }
- }
-}
-
/**
* Run the triggers registered on commit of a change in _space.
*/
@@ -1753,21 +1726,6 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
txn_on_rollback(txn, on_rollback_view);
select_guard.is_active = false;
}
- /* Setup SQL-actions trigger if required. */
- if (def->opts.checks_ast != NULL) {
- struct trigger *sql_actions_trigger =
- (struct trigger *)malloc(sizeof(struct trigger));
- if (sql_actions_trigger == NULL) {
- tnt_raise(OutOfMemory, sizeof(struct trigger),
- "calloc",
- "on_space_before_replace");
- }
- trigger_create(sql_actions_trigger,
- on_space_before_replace,
- (void *)(uintptr_t)space->def->id,
- (trigger_f0)free);
- trigger_add(&space->before_replace, sql_actions_trigger);
- }
} else if (new_tuple == NULL) { /* DELETE */
access_check_ddl(old_space->def->name, old_space->def->id,
old_space->def->uid, SC_SPACE, PRIV_D);
diff --git a/src/box/space.c b/src/box/space.c
index 6da0e6e9a..b139a0905 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -109,6 +109,33 @@ space_fill_index_map(struct space *space)
}
}
+/**
+ * SQL-specific actions for space.
+ */
+static void
+on_space_before_replace(struct trigger *trigger, void *event)
+{
+ uint32_t space_id = (uint32_t)(uintptr_t)trigger->data;
+ struct space *space = space_cache_find(space_id);
+ assert(space != NULL);
+ struct txn *txn = (struct txn *) event;
+ struct txn_stmt *stmt = txn_current_stmt(txn);
+ if (stmt == NULL)
+ return;
+ struct tuple *new_tuple = stmt->new_tuple;
+ struct tuple *old_tuple = stmt->old_tuple;
+ if (new_tuple != NULL && space->sql_checks != NULL) {
+ const char *new_tuple_raw = tuple_data(new_tuple);
+ const char *old_tuple_raw = old_tuple != NULL ?
+ tuple_data(old_tuple) : NULL;
+ if (sql_checks_run(space->sql_checks,
+ space->def->opts.checks_ast, space->def,
+ old_tuple_raw, new_tuple_raw) != 0) {
+ diag_raise();
+ }
+ }
+}
+
int
space_create(struct space *space, struct engine *engine,
const struct space_vtab *vtab, struct space_def *def,
@@ -172,6 +199,18 @@ space_create(struct space *space, struct engine *engine,
if (unlikely(checks == NULL))
goto fail_free_indexes;
space->sql_checks = checks;
+
+ struct trigger *sql_actions_trigger =
+ (struct trigger *)malloc(sizeof(struct trigger));
+ if (sql_actions_trigger == NULL) {
+ diag_set(OutOfMemory, sizeof(struct trigger),
+ "calloc", "on_space_before_replace");
+ goto fail_free_indexes;
+ }
+ trigger_create(sql_actions_trigger, on_space_before_replace,
+ (void *)(uintptr_t)space->def->id,
+ (trigger_f0)free);
+ trigger_add(&space->before_replace, sql_actions_trigger);
}
return 0;
^ permalink raw reply [flat|nested] 9+ messages in thread