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 BD5B724523 for ; Thu, 10 Jan 2019 08:54:55 -0500 (EST) 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 gGRTqe6BOoKv for ; Thu, 10 Jan 2019 08:54:55 -0500 (EST) Received: from smtp55.i.mail.ru (smtp55.i.mail.ru [217.69.128.35]) (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 95D22245A4 for ; Thu, 10 Jan 2019 08:54:54 -0500 (EST) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v1 4/4] sql: make sql checks on server side Date: Thu, 10 Jan 2019 16:54:50 +0300 Message-Id: <2d7c73e83bcee024ffcc095854a31bcd46f6ccab.1547128310.git.kshcherbatov@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 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/space.c | 49 +++++++ src/box/space.h | 5 + src/box/sql.c | 177 +++++++++++++++++++++++++- src/box/sql.h | 66 ++++++++++ src/box/sql/build.c | 2 +- src/box/sql/insert.c | 107 ++++------------ src/box/sql/sqliteInt.h | 32 +++++ src/box/sql/vdbe.c | 2 +- src/box/sql/vdbeapi.c | 41 ++---- test/sql-tap/check.test.lua | 24 ++-- test/sql-tap/fkey2.test.lua | 4 +- test/sql-tap/subquery.test.lua | 2 +- 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, 461 insertions(+), 135 deletions(-) diff --git a/src/box/space.c b/src/box/space.c index 4d174f7ec..b139a0905 100644 --- a/src/box/space.c +++ b/src/box/space.c @@ -29,6 +29,7 @@ * SUCH DAMAGE. */ #include "space.h" +#include "sql.h" #include #include #include @@ -108,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, @@ -165,6 +193,25 @@ 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; + + 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; fail_free_indexes: @@ -216,6 +263,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 26b84c5db..8961c4fd9 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -35,9 +35,10 @@ #include "sql/tarantoolInt.h" #include "sql/vdbeInt.h" #include "mpstream.h" +#include "execute.h" #include "index.h" -#include +#include "info.h" #include "schema.h" #include "box.h" #include "txn.h" @@ -1377,3 +1378,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. */ + sql_stmt_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 028a15245..f2a6587da 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 @@ -411,6 +434,49 @@ void sqlite3SrcListDelete(struct sqlite3 *db, struct SrcList *list); +/** + * 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 9c6b1b49a..40675fd32 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -873,7 +873,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 f147f6a50..ab0e12def 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); /* * Other actions except for REPLACE and UPDATE OR IGNORE diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 7e16edc9a..5a11d9d13 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -594,6 +594,19 @@ 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(). + * @param stmt VDBE program, may be NULL. + * @retval SQLITE_OK on success, sql_ret_code error code. + */ +int +sql_stmt_reset(struct sqlite3_stmt *stmt); + int sqlite3_exec(sqlite3 *, /* An open database */ const char *sql, /* SQL to be evaluated */ @@ -3869,6 +3882,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/vdbe.c b/src/box/sql/vdbe.c index 9f9d0eacf..74a2e366c 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -992,7 +992,7 @@ case OP_HaltIfNull: { /* in3 */ * automatically. * * P1 is the result code returned by sqlite3_exec(), - * sqlite3_reset(), or sqlite3_finalize(). For a normal halt, + * sql_stmt_reset(), or sqlite3_finalize(). For a normal halt, * this should be SQLITE_OK (0). * For errors, it can be some other value. If P1!=0 then P2 will * determine whether or not to rollback the current transaction. diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index 9e57af051..b62de9d39 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -132,23 +132,15 @@ 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) +sql_stmt_reset(struct sqlite3_stmt *stmt) { int rc; - if (pStmt == 0) { + if (stmt == NULL) { rc = SQLITE_OK; } else { - Vdbe *v = (Vdbe *) pStmt; - sqlite3 *db = v->db; + struct Vdbe *v = (struct Vdbe *)stmt; + struct sqlite3 *db = v->db; checkProfileCallback(db, v); rc = sqlite3VdbeReset(v); sqlite3VdbeRewind(v); @@ -509,30 +501,19 @@ sqlite3Step(Vdbe * p) assert(p); if (p->magic != VDBE_MAGIC_RUN) { - /* We used to require that sqlite3_reset() be called before retrying - * sqlite3_step() after any error or after SQLITE_DONE. But beginning - * with version 3.7.0, we changed this so that sqlite3_reset() would - * be called automatically instead of throwing the SQLITE_MISUSE error. - * This "automatic-reset" change is not technically an incompatibility, - * since any application that receives an SQLITE_MISUSE is broken by - * definition. - * - * Nevertheless, some published applications that were originally written - * for version 3.6.23 or earlier do in fact depend on SQLITE_MISUSE - * returns, and those were broken by the automatic-reset change. As a - * a work-around, the SQLITE_OMIT_AUTORESET compile-time restores the - * legacy behavior of returning SQLITE_MISUSE for cases where the - * previous sqlite3_step() returned something other than a SQLITE_LOCKED - * or SQLITE_BUSY error. + /* + * The sql_stmt_reset() routine would be called + * automatically instead of throwing the + * SQLITE_MISUSE error. */ #ifdef SQLITE_OMIT_AUTORESET if ((rc = p->rc & 0xff) == SQLITE_BUSY || rc == SQLITE_LOCKED) { - sqlite3_reset((sqlite3_stmt *) p); + sql_stmt_reset((sqlite3_stmt *) p); } else { return SQLITE_MISUSE_BKPT; } #else - sqlite3_reset((sqlite3_stmt *) p); + sql_stmt_reset((sqlite3_stmt *) p); #endif } @@ -632,7 +613,7 @@ sqlite3_step(sqlite3_stmt * pStmt) rc2 = rc = sqlite3Reprepare(v); if (rc != SQLITE_OK) break; - sqlite3_reset(pStmt); + sql_stmt_reset(pStmt); if (savedPc >= 0) v->doingRerun = 1; assert(v->expired == 0); diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua index 1176ad500..08b7edcc7 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); ]], { -- - 1, "CHECK constraint failed: T1" + 1, "Failed to execute SQL statement: CHECK constraint failed: T1" -- }) @@ -75,7 +75,7 @@ test:do_catchsql_test( INSERT INTO t1 VALUES(4,3, 2); ]], { -- - 1, "CHECK constraint failed: T1" + 1, "Failed to execute SQL statement: CHECK constraint failed: T1" -- }) @@ -147,7 +147,7 @@ test:do_catchsql_test( UPDATE t1 SET x=7 WHERE x==2 ]], { -- - 1, "CHECK constraint failed: T1" + 1, "Failed to execute SQL statement: CHECK constraint failed: T1" -- }) @@ -167,7 +167,7 @@ test:do_catchsql_test( UPDATE t1 SET x=5 WHERE x==2 ]], { -- - 1, "CHECK constraint failed: T1" + 1, "Failed to execute SQL statement: CHECK constraint failed: T1" -- }) @@ -330,7 +330,7 @@ test:do_catchsql_test( INSERT INTO t3 VALUES(111,222,333); ]], { -- - 1, "CHECK constraint failed: T3" + 1, "Failed to execute SQL statement: CHECK constraint failed: T3" -- }) @@ -401,7 +401,7 @@ test:do_catchsql_test( UPDATE t4 SET x=0, y=1; ]], { -- - 1, "CHECK constraint failed: T4" + 1, "Failed to execute SQL statement: CHECK constraint failed: T4" -- }) @@ -421,7 +421,7 @@ test:do_catchsql_test( UPDATE t4 SET x=0, y=2; ]], { -- - 1, "CHECK constraint failed: T4" + 1, "Failed to execute SQL statement: CHECK constraint failed: T4" -- }) @@ -498,7 +498,7 @@ test:do_catchsql_test( UPDATE OR FAIL t1 SET x=7-x, y=y+1; ]], { -- - 1, "CHECK constraint failed: T1" + 1, "Failed to execute SQL statement: CHECK constraint failed: T1" -- }) @@ -520,7 +520,7 @@ test:do_catchsql_test( INSERT OR ROLLBACK INTO t1 VALUES(8,40.0, 10); ]], { -- - 1, "CHECK constraint failed: T1" + 1, "Failed to execute SQL statement: CHECK constraint failed: T1" -- }) @@ -553,7 +553,7 @@ test:do_catchsql_test( REPLACE INTO t1 VALUES(6,7, 11); ]], { -- - 1, "CHECK constraint failed: T1" + 1, "Failed to execute SQL statement: CHECK constraint failed: T1" -- }) @@ -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" -- }) @@ -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" -- }) end diff --git a/test/sql-tap/fkey2.test.lua b/test/sql-tap/fkey2.test.lua index 03bf025f3..1a22b5b55 100755 --- a/test/sql-tap/fkey2.test.lua +++ b/test/sql-tap/fkey2.test.lua @@ -362,7 +362,7 @@ test:do_catchsql_test( UPDATE ab SET a = 5; ]], { -- - 1, "CHECK constraint failed: EF" + 1, "Failed to execute SQL statement: CHECK constraint failed: EF" -- }) @@ -382,7 +382,7 @@ test:do_catchsql_test( UPDATE ab SET a = 5; ]], { -- - 1, "CHECK constraint failed: EF" + 1, "Failed to execute SQL statement: CHECK constraint failed: EF" -- }) diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua index fb9a737d1..38988dbdc 100755 --- a/test/sql-tap/subquery.test.lua +++ b/test/sql-tap/subquery.test.lua @@ -651,7 +651,7 @@ test:do_execsql_test( -------------------------------------------------------------------- -- These tests - subquery-4.* - use the TCL statement cache to try -- and expose bugs to do with re-using statements that have been --- passed to sqlite3_reset(). +-- passed to sql_stmt_reset(). -- -- One problem was that VDBE memory cells were not being initialized -- to NULL on the second and subsequent executions. diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua index 7057f6b0f..43f8fc4cb 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); ]], { -- - 1, "CHECK constraint failed: T21" + 1, "Failed to execute SQL statement: CHECK constraint failed: T21" -- }) @@ -1227,7 +1227,7 @@ test:do_catchsql_test( INSERT INTO T21 VALUES(1, 1, -1); ]], { -- - 1, "CHECK constraint failed: T21" + 1, "Failed to execute SQL statement: CHECK constraint failed: T21" -- }) @@ -1368,7 +1368,7 @@ test:do_catchsql_test( INSERT INTO T28 VALUES(0); ]], { -- - 1, "CHECK constraint failed: CHECK1" + 1, "Failed to execute SQL statement: CHECK constraint failed: CHECK1" -- }) @@ -1378,7 +1378,7 @@ test:do_catchsql_test( INSERT INTO T28 VALUES(9); ]], { -- - 1, "CHECK constraint failed: CHECK2" + 1, "Failed to execute SQL statement: CHECK constraint failed: CHECK2" -- }) 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