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 B636A2AC53 for ; Tue, 26 Mar 2019 06:59:31 -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 bscQmMLx5xC8 for ; Tue, 26 Mar 2019 06:59:31 -0400 (EDT) Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (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 4ACAC2AC24 for ; Tue, 26 Mar 2019 06:59:31 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 9/9] sql: run check constraint tests on space alter References: <9ba32d3784fdf28096b8ccde7d5fb3c6a55c0826.1548838034.git.kshcherbatov@tarantool.org> From: Kirill Shcherbatov Message-ID: <383ecdc1-35d8-7a15-fe7c-e1cdba941dfd@tarantool.org> Date: Tue, 26 Mar 2019 13:59:29 +0300 MIME-Version: 1.0 In-Reply-To: <9ba32d3784fdf28096b8ccde7d5fb3c6a55c0826.1548838034.git.kshcherbatov@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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, korablev@tarantool.org Introduced reusable pre-compiled VDBE programs for ck constraints and space trigger to fire checks on insert and update operations. Closes #3691 @TarantoolBot document Title: check constraint for LUA space Now it is possible to create check constraints for LUA spaces: insert the tuple {, , } in the _ck_constraint space to create new check constraint. Example: s = box.schema.create_space('test') s:create_index('pk') format = {{'X', 'unsigned'}, {'Y', 'unsigned'}} s:format(format) box.space._ck_constraint:insert({'physics', s.id, 'Xnew_space->ck_constraint, &ck_constraint); rlist_swap(&ck_constraint, &alter->old_space->ck_constraint); + struct ck_constraint *ck; + rlist_foreach_entry(ck, &ck_constraint, link) + trigger_clear(&ck->trigger); + rlist_foreach_entry(ck, &alter->new_space->ck_constraint, link) + trigger_add(&alter->new_space->before_replace, &ck->trigger); } void @@ -1386,6 +1391,11 @@ BuildCkConstraints::rollback(struct alter_space *alter) { rlist_swap(&alter->old_space->ck_constraint, &ck_constraint); rlist_swap(&ck_constraint, &alter->new_space->ck_constraint); + struct ck_constraint *ck; + rlist_foreach_entry(ck, &ck_constraint, link) + trigger_clear(&ck->trigger); + rlist_foreach_entry(ck, &alter->old_space->ck_constraint, link) + trigger_add(&alter->old_space->before_replace, &ck->trigger); } BuildCkConstraints::~BuildCkConstraints() @@ -4166,10 +4176,12 @@ on_replace_ck_constraint_rollback(struct trigger *trigger, void *event) return; assert(space != NULL); rlist_add_entry(&space->ck_constraint, ck_constraint, link); + trigger_add(&space->before_replace, &ck_constraint->trigger); } else if (stmt->new_tuple != NULL && stmt->old_tuple == NULL) { /* Rollback INSERT check constraint. */ assert(space != NULL); rlist_del_entry(ck_constraint, link); + trigger_clear(&ck_constraint->trigger); ck_constraint_delete(ck_constraint); } else { /* Rollback REPLACE check constraint. */ @@ -4180,7 +4192,9 @@ on_replace_ck_constraint_rollback(struct trigger *trigger, void *event) strlen(space_name)); assert(new_ck_constraint != NULL); rlist_del_entry(new_ck_constraint, link); + trigger_clear(&new_ck_constraint->trigger); rlist_add_entry(&space->ck_constraint, ck_constraint, link); + trigger_add(&space->before_replace, &ck_constraint->trigger); ck_constraint_delete(new_ck_constraint); } } @@ -4232,9 +4246,13 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event) struct ck_constraint *old_ck_constraint = space_ck_constraint_by_name(space, space_name, strlen(space_name)); - if (old_ck_constraint != NULL) + if (old_ck_constraint != NULL) { rlist_del_entry(old_ck_constraint, link); + trigger_clear(&old_ck_constraint->trigger); + } rlist_add_entry(&space->ck_constraint, new_ck_constraint, link); + trigger_add(&space->before_replace, + &new_ck_constraint->trigger); on_commit->data = old_ck_constraint; on_rollback->data = old_tuple == NULL ? new_ck_constraint : old_ck_constraint; @@ -4250,6 +4268,7 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event) space_ck_constraint_by_name(space, name, name_len); assert(old_ck_constraint != NULL); rlist_del_entry(old_ck_constraint, link); + trigger_clear(&old_ck_constraint->trigger); on_commit->data = old_ck_constraint; on_rollback->data = old_ck_constraint; } diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c index 110098efc..c2e8547f1 100644 --- a/src/box/ck_constraint.c +++ b/src/box/ck_constraint.c @@ -29,11 +29,16 @@ * SUCH DAMAGE. */ #include +#include "bind.h" #include "ck_constraint.h" #include "errcode.h" +#include "session.h" +#include "schema.h" #include "small/rlist.h" +#include "tuple.h" #include "sql.h" #include "sql/sqlInt.h" +#include "sql/vdbeInt.h" /** * Resolve space_def references for check constraint via AST @@ -84,6 +89,130 @@ ck_constraint_def_create(struct ck_constraint_def *ck_constraint_def, rlist_create(&ck_constraint_def->link); } +/** + * Compile constraint check subroutine. + * @param ck_constraint Check constraint to compile. + * @param expr Check constraint expression AST is built for + * ck_constraint->def. + * @param space_def The space definition of the space this check + * constraint is constructed for. + * @retval not NULL sql_stmt program pointer on success. + * @retval NULL otherwise. + */ +int +ck_constraint_test_compile(struct ck_constraint *ck_constraint, + struct Expr *expr, const struct space_def *space_def) +{ + int rc = -1; + assert(ck_constraint->space_id == space_def->id); + struct Parse parser; + sql_parser_create(&parser, sql_get()); + struct Vdbe *v = sqlGetVdbe(&parser); + if (v == NULL) { + diag_set(OutOfMemory, sizeof(struct Vdbe), + "sqlGetVdbe", "vdbe"); + goto end; + } + + /* 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_flags; + + /* + * Generate a prologue code to bind variable new_tuple_var + * to new_tuple_reg. + */ + uint32_t field_count = space_def->field_count; + int new_tuple_reg = sqlGetTempRange(&parser, field_count); + struct Expr bind = {.op = TK_VARIABLE, .u.zToken = "?"}; + ck_constraint->new_tuple_var = parser.nVar + 1; + for (uint32_t i = 0; i < field_count; i++) { + sqlExprAssignVarNumber(&parser, &bind, 1); + sqlExprCodeTarget(&parser, &bind, new_tuple_reg + i); + } + vdbe_emit_ck_constraint(&parser, expr, ck_constraint->def->name, + new_tuple_reg); + sql_finish_coding(&parser); + if (parser.is_aborted) { + diag_set(ClientError, ER_CREATE_CK_CONSTRAINT, + ck_constraint->def->name, + "can not compile expression"); + goto end; + } + sql_parser_destroy(&parser); + + /* Restore original sql flags for user_session. */ + user_session->sql_flags = sql_flags; + ck_constraint->stmt = (struct sql_stmt *)v; + rc = 0; +end: + return rc; +} + +/** + * Perform ck constraint checks with new tuple data new_tuple_raw + * before insert or replace in space space_def. + * @param ck_constraint Check constraint to test. + * @param space_def The space definition of the space this check + * constraint is constructed for. + * @param new_tuple_raw The tuple to be inserted in space. + * @retval 0 if check constraint test is passed, -1 otherwise. + */ +static int +ck_constraint_test(struct ck_constraint *ck_constraint, + struct space_def *space_def, const char *new_tuple_raw) +{ + assert(new_tuple_raw != NULL); + /* + * 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] + */ + mp_decode_array(&new_tuple_raw); + /* Reset VDBE to make new bindings. */ + sql_reset(ck_constraint->stmt); + for (uint32_t i = 0; i < space_def->field_count; i++) { + struct sql_bind bind; + if (sql_bind_decode(&bind, ck_constraint->new_tuple_var + i, + &new_tuple_raw) != 0) + return -1; + if (sql_bind_column(ck_constraint->stmt, &bind, + ck_constraint->new_tuple_var + i) != 0) + return -1; + } + /* Checks VDBE can't expire, reset expired flag & Burn. */ + struct Vdbe *v = (struct Vdbe *)ck_constraint->stmt; + v->expired = 0; + int rc; + while ((rc = sql_step(ck_constraint->stmt)) == SQL_ROW) {} + if (v->rc != SQL_DONE && v->rc != SQL_TARANTOOL_ERROR) + diag_set(ClientError, ER_SQL, v->zErrMsg); + return rc == SQL_DONE ? 0 : -1; +} + +/** + * Trigger routine executing ck constraint check on space + * insert and replace. + */ +static void +ck_constraint_space_trigger(struct trigger *trigger, void *event) +{ + struct ck_constraint *ck_constraint = + (struct ck_constraint *)trigger->data; + struct space *space = space_by_id(ck_constraint->space_id); + assert(space != NULL); + struct txn *txn = (struct txn *) event; + struct txn_stmt *stmt = txn_current_stmt(txn); + struct tuple *new_tuple = stmt->new_tuple; + if (stmt == NULL || new_tuple == NULL) + return; + if (ck_constraint_test(ck_constraint, space->def, + tuple_data(new_tuple)) != 0) + diag_raise(); +} + struct ck_constraint * ck_constraint_new(const struct ck_constraint_def *ck_constraint_def, struct space_def *space_def) @@ -110,6 +239,8 @@ ck_constraint_new(const struct ck_constraint_def *ck_constraint_def, ck_constraint_def_create(ck_constraint->def, ck_constraint_def->name, ck_constraint_name_len, ck_constraint_def->expr_str, expr_str_len); + trigger_create(&ck_constraint->trigger, ck_constraint_space_trigger, + ck_constraint, NULL); struct Expr *expr = sql_expr_compile(sql_get(), ck_constraint_def->expr_str, expr_str_len); @@ -120,18 +251,23 @@ ck_constraint_new(const struct ck_constraint_def *ck_constraint_def, box_error_message(box_error_last())); goto error; } - ck_constraint->expr = expr; + if (ck_constraint_test_compile(ck_constraint, expr, space_def) != 0) + goto error; +end: + sql_expr_delete(sql_get(), expr, false); return ck_constraint; error: ck_constraint_delete(ck_constraint); - return NULL; + ck_constraint = NULL; + goto end; } void ck_constraint_delete(struct ck_constraint *ck_constraint) { - sql_expr_delete(sql_get(), ck_constraint->expr, false); + assert(rlist_empty(&ck_constraint->trigger.link)); + sql_finalize(ck_constraint->stmt); TRASH(ck_constraint); free(ck_constraint); } diff --git a/src/box/ck_constraint.h b/src/box/ck_constraint.h index 02aa525ce..77b1a9bed 100644 --- a/src/box/ck_constraint.h +++ b/src/box/ck_constraint.h @@ -32,6 +32,7 @@ */ #include +#include "trigger.h" #include "small/rlist.h" #if defined(__cplusplus) @@ -40,6 +41,7 @@ extern "C" { struct space; struct space_def; +struct sql_stmt; struct Expr; /** @@ -72,17 +74,29 @@ struct ck_constraint { */ struct ck_constraint_def *def; /** - * The check constraint expression AST is built for - * ck_constraint::def::expr_str with sql_expr_compile - * and resolved with sqlResolveExprNames for - * space with space[ck_constraint::space_id] definition. + * Precompiled reusable VDBE program for proceeding ck + * constraint checks and setting bad exitcode and error + * message when ck condition unsatisfied. + * Program rely on new_tuple_var parameter to be binded + * in the VDBE memory before run. */ - struct Expr *expr; + struct sql_stmt *stmt; /** * The id of the space this check constraint is * built for. */ uint32_t space_id; + /** + * The first ck_constraint::stmt VDBE variable of the + * range space[ck_constraint::space_id]->def->field_count + * representing a new tuple to be inserted. + */ + int new_tuple_var; + /** + * Trigger object executing check constraint on space + * insert and replace. + */ + struct trigger trigger; /** * Organize check constraint structs into linked list * with space::ck_constraint. diff --git a/src/box/sql.c b/src/box/sql.c index fc469126e..912b24bf0 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -29,6 +29,7 @@ * SUCH DAMAGE. */ #include +#include "bind.h" #include "field_def.h" #include "cfg.h" #include "sql.h" diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c index 2fe74a027..3caad3c24 100644 --- a/src/box/sql/insert.c +++ b/src/box/sql/insert.c @@ -798,51 +798,26 @@ sqlInsert(Parse * pParse, /* Parser context */ sqlDbFree(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) -{ - 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; - } - } - } - 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) +void +vdbe_emit_ck_constraint(struct Parse *parser, struct Expr *expr, + const char *name, int new_tuple_reg) { - Walker w; - memset(&w, 0, sizeof(w)); - w.eCode = 0; - w.xExprCallback = checkConstraintExprNode; - w.u.aiCol = aiChng; - sqlWalkExpr(&w, pExpr); - testcase(w.eCode == 0); - testcase(w.eCode == CKCNSTRNT_COLUMN); - return !w.eCode; + parser->ckBase = new_tuple_reg; + struct Vdbe *v = sqlGetVdbe(parser); + const char *ck_constraint_name = sqlDbStrDup(parser->db, name); + VdbeNoopComment((v, "BEGIN: ck constraint %s test", name)); + /* Skip check when it is turned off. */ + int all_is_ok = sqlVdbeMakeLabel(v); + sqlExprIfTrue(parser, expr, all_is_ok, SQL_JUMPIFNULL); + sqlMayAbort(parser); + const char *fmt = tnt_errcode_desc(ER_CK_CONSTRAINT_FAILED); + const char *error_msg = tt_sprintf(fmt, ck_constraint_name); + sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, + ON_CONFLICT_ACTION_ABORT, 0, + sqlDbStrDup(parser->db, error_msg), P4_DYNAMIC); + sqlVdbeChangeP5(v, ER_CK_CONSTRAINT_FAILED); + VdbeNoopComment((v, "END: ck constraint %s test", name)); + sqlVdbeResolveLabel(v, all_is_ok); } void @@ -912,37 +887,6 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space, 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. */ - enum on_conflict_action on_conflict_check = on_conflict; - if (on_conflict == ON_CONFLICT_ACTION_REPLACE) - on_conflict_check = ON_CONFLICT_ACTION_ABORT; - if (!rlist_empty(&space->ck_constraint)) - parse_context->ckBase = new_tuple_reg; - struct ck_constraint *ck_constraint; - rlist_foreach_entry(ck_constraint, &space->ck_constraint, link) { - struct Expr *expr = ck_constraint->expr; - if (is_update && checkConstraintUnchanged(expr, upd_cols) != 0) - continue; - int all_ok = sqlVdbeMakeLabel(v); - sqlExprIfTrue(parse_context, expr, all_ok, SQL_JUMPIFNULL); - if (on_conflict == ON_CONFLICT_ACTION_IGNORE) { - sqlVdbeGoto(v, ignore_label); - sqlVdbeResolveLabel(v, all_ok); - } else { - char *name = ck_constraint->def->name; - sqlHaltConstraint(parse_context, - SQL_CONSTRAINT_CHECK, - on_conflict_check, name, - P4_TRANSIENT, P5_ConstraintCheck); - } - sqlVdbeResolveLabel(v, all_ok); - } sql_emit_table_types(v, space->def, new_tuple_reg); /* * Other actions except for REPLACE and UPDATE OR IGNORE diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 9fef9ea2e..2a79d2b59 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -592,6 +592,17 @@ sql_column_value(sql_stmt *, int sql_finalize(sql_stmt * pStmt); +/* + * Terminate the current execution of an SQL statement and reset + * it back to its starting state so that it can be reused. + * + * @param stmt VDBE program, may be NULL. + * @retval SQL_OK On success. + * @retval sql_ret_code Error code on error. + */ +int +sql_reset(struct sql_stmt *stmt); + int sql_exec(sql *, /* An open database */ const char *sql, /* SQL to be evaluated */ @@ -3827,6 +3838,20 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, enum on_conflict_action on_conflict, int ignore_label, int *upd_cols); +/** + * Gnerate code to make check constraints tests on tuple insertion + * on INSERT, REPLACE or UPDATE operations. + * @param parser Current parsing context. + * @param expr Check constraint AST. + * @param name Check constraint name to raise error. + * @param new_tuple_reg The first ck_constraint::stmt VDBE + * register of the range + * space_def::field_count representing a + * new tuple to be inserted. + */ +void +vdbe_emit_ck_constraint(struct Parse *parser, struct Expr *expr, + const char *name, int new_tuple_reg); /** * This routine generates code to finish the INSERT or UPDATE * operation that was started by a prior call to diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index be5c9dff9..ebedd3c42 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -132,14 +132,6 @@ sql_finalize(sql_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 - * sql_errcode(), sql_errmsg() and sql_errmsg16(). - */ int sql_reset(sql_stmt * pStmt) { diff --git a/test/sql/checks.result b/test/sql/checks.result index aeacd09cb..2d46a2edc 100644 --- a/test/sql/checks.result +++ b/test/sql/checks.result @@ -71,6 +71,10 @@ box.sql.execute("INSERT INTO \"test\" VALUES(5);") --- - error: 'Check constraint failed: CK_CONSTRAINT_01' ... +box.space.test:insert({5}) +--- +- error: 'Check constraint failed: CK_CONSTRAINT_01' +... box.space._ck_constraint:replace({'CK_CONSTRAINT_01', 513, 'X<=5'}) --- - ['CK_CONSTRAINT_01', 513, 'X<=5'] @@ -82,6 +86,10 @@ box.sql.execute("INSERT INTO \"test\" VALUES(6);") --- - error: 'Check constraint failed: CK_CONSTRAINT_01' ... +box.space.test:insert({6}) +--- +- error: 'Check constraint failed: CK_CONSTRAINT_01' +... -- Can't drop table with check constraints. box.space.test:delete({5}) --- @@ -113,13 +121,25 @@ box.sql.execute("INSERT INTO t1 VALUES (7, 1, 1)") --- - error: 'Check constraint failed: ONE' ... +box.space.T1:insert({7, 1, 1}) +--- +- error: 'Check constraint failed: ONE' +... box.sql.execute("INSERT INTO t1 VALUES (2, 1, 1)") --- - error: 'Check constraint failed: TWO' ... +box.space.T1:insert({2, 1, 1}) +--- +- error: 'Check constraint failed: TWO' +... box.sql.execute("INSERT INTO t1 VALUES (2, 4, 1)") --- ... +box.space.T1:update({1}, {{'+', 1, 5}}) +--- +- error: 'Check constraint failed: ONE' +... box.sql.execute("DROP TABLE t1") --- ... diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua index 9a7e5faf4..0e6c990c9 100644 --- a/test/sql/checks.test.lua +++ b/test/sql/checks.test.lua @@ -27,9 +27,11 @@ box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, 'X<5'}) box.space._ck_constraint:count({}) box.sql.execute("INSERT INTO \"test\" VALUES(5);") +box.space.test:insert({5}) box.space._ck_constraint:replace({'CK_CONSTRAINT_01', 513, 'X<=5'}) box.sql.execute("INSERT INTO \"test\" VALUES(5);") box.sql.execute("INSERT INTO \"test\" VALUES(6);") +box.space.test:insert({6}) -- Can't drop table with check constraints. box.space.test:delete({5}) box.space.test.index.pk:drop() @@ -41,8 +43,11 @@ box.space.test:drop() 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._ck_constraint:count() box.sql.execute("INSERT INTO t1 VALUES (7, 1, 1)") +box.space.T1:insert({7, 1, 1}) box.sql.execute("INSERT INTO t1 VALUES (2, 1, 1)") +box.space.T1:insert({2, 1, 1}) box.sql.execute("INSERT INTO t1 VALUES (2, 4, 1)") +box.space.T1:update({1}, {{'+', 1, 5}}) box.sql.execute("DROP TABLE t1") -- -- 2.21.0