Tarantool development patches archive
 help / color / mirror / Atom feed
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
To: tarantool-patches@freelists.org, korablev@tarantool.org
Cc: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Subject: [tarantool-patches] [PATCH v1 4/4] sql: make sql checks on server side
Date: Thu, 10 Jan 2019 16:54:50 +0300	[thread overview]
Message-ID: <2d7c73e83bcee024ffcc095854a31bcd46f6ccab.1547128310.git.kshcherbatov@tarantool.org> (raw)
In-Reply-To: <cover.1547128310.git.kshcherbatov@tarantool.org>

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 <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
@@ -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 <info.h>
+#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);
     ]], {
         -- <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 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;
     ]], {
         -- <fkey2-3.2>
-        1, "CHECK constraint failed: EF"
+        1, "Failed to execute SQL statement: CHECK constraint failed: EF"
         -- </fkey2-3.2>
     })
 
@@ -382,7 +382,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/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);
     ]], {
         -- <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

  parent reply	other threads:[~2019-01-10 13:54 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-10 13:54 [tarantool-patches] [PATCH v1 0/4] sql: Checks " Kirill Shcherbatov
2019-01-10 13:54 ` [tarantool-patches] [PATCH v1 1/4] box: rename space->opts checks to checks_ast Kirill Shcherbatov
2019-01-11 14:05   ` [tarantool-patches] " Konstantin Osipov
2019-01-18 18:00   ` Konstantin Osipov
2019-01-10 13:54 ` [tarantool-patches] [PATCH v1 2/4] sql: disallow use of TYPEOF in Check Kirill Shcherbatov
2019-01-11 14:06   ` [tarantool-patches] " Konstantin Osipov
2019-01-11 14:07   ` Konstantin Osipov
2019-01-18 18:04   ` Konstantin Osipov
2019-01-10 13:54 ` [tarantool-patches] [PATCH v1 3/4] box: exported sql_bind structure and API Kirill Shcherbatov
2019-01-10 13:54 ` Kirill Shcherbatov [this message]
2019-01-11 14:12   ` [tarantool-patches] Re: [PATCH v1 4/4] sql: make sql checks on server side Konstantin Osipov
2019-01-11 14:14   ` Konstantin Osipov
2019-01-18 18:11   ` Konstantin Osipov
2019-01-21 14:47   ` n.pettik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2d7c73e83bcee024ffcc095854a31bcd46f6ccab.1547128310.git.kshcherbatov@tarantool.org \
    --to=kshcherbatov@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH v1 4/4] sql: make sql checks on server side' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox