* [tarantool-patches] [PATCH v4 0/4] box: run checks on insertions in LUA spaces
@ 2019-05-16 13:56 Kirill Shcherbatov
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 1/4] schema: add new system space for CHECK constraints Kirill Shcherbatov
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-16 13:56 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
Fire CK constraints for LUA spaces.
To achieve this goal, we reworked data dictionary, to store ck
constraints in separate space _ck_constraints and updated data
migration script to migrate existent data there. This also would
be useful in future to implement ALTER SPACE ADD CONSTRAINT
operation. Now we do not support CK constraint creation on
non-empty space.
Each CK has own precompiled VDBE machine that performs this
check with tuple fields mapped to it's memory with sql_bind() api.
In case of ck constraint conflict detected by this VM we abort
the transaction and return error to user.
Finally, we introduced a LUA-wrapper that provide a user-friendly
way to manage space ck constraints.
Changes in version 4:
- language field in _ck_constraint space
- moved sql_flags context to parser and VDBE
- cool column_mask binding optimization
v2: https://www.freelists.org/post/tarantool-patches/PATCH-v2-09-sql-Checks-on-server-side
v1: https://www.freelists.org/post/tarantool-patches/PATCH-v1-04-sql-Checks-on-server-side
Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3691-checks-on-server-side
Issue: https://github.com/tarantool/tarantool/issues/3691
Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-3691-checks-on-server-side
Issue: https://github.com/tarantool/tarantool/issues/3691
Kirill Shcherbatov (4):
schema: add new system space for CHECK constraints
box: run check constraint tests on space alter
box: introduce column_mask for ck constraint
box: user-friendly interface to manage ck constraints
src/box/CMakeLists.txt | 1 +
src/box/alter.cc | 313 ++++++++++++++-
src/box/alter.h | 1 +
src/box/bootstrap.snap | Bin 4374 -> 4428 bytes
src/box/ck_constraint.c | 313 +++++++++++++++
src/box/ck_constraint.h | 186 +++++++++
src/box/errcode.h | 4 +-
src/box/lua/schema.lua | 39 +-
src/box/lua/space.cc | 65 +++
src/box/lua/upgrade.lua | 43 ++
src/box/schema.cc | 8 +
src/box/schema_def.h | 11 +
src/box/space.c | 2 +
src/box/space.h | 5 +
src/box/space_def.c | 98 +----
src/box/space_def.h | 2 -
src/box/sql.c | 86 +---
src/box/sql.h | 41 +-
src/box/sql/build.c | 213 ++++++++--
src/box/sql/insert.c | 115 ++----
src/box/sql/parse.y | 2 +-
src/box/sql/parse_def.h | 24 ++
src/box/sql/resolve.c | 19 +-
src/box/sql/select.c | 11 +-
src/box/sql/sqlInt.h | 34 +-
src/box/sql/tokenize.c | 1 -
src/box/sql/vdbeapi.c | 8 -
test/app-tap/tarantoolctl.test.lua | 4 +-
test/box-py/bootstrap.result | 7 +-
test/box/access.result | 3 +
test/box/access.test.lua | 1 +
test/box/access_misc.result | 3 +
test/box/access_sysview.result | 6 +-
test/box/alter.result | 6 +-
test/box/misc.result | 2 +
test/sql-tap/check.test.lua | 42 +-
test/sql-tap/fkey2.test.lua | 4 +-
test/sql-tap/sql-errors.test.lua | 2 +-
test/sql-tap/table.test.lua | 12 +-
test/sql/checks.result | 547 ++++++++++++++++++++++++--
test/sql/checks.test.lua | 204 ++++++++--
test/sql/errinj.result | 140 +++++++
test/sql/errinj.test.lua | 45 +++
test/sql/gh-2981-check-autoinc.result | 12 +-
test/sql/types.result | 3 +-
test/sql/upgrade.result | 19 +
test/sql/upgrade.test.lua | 5 +
test/wal_off/alter.result | 2 +-
48 files changed, 2222 insertions(+), 492 deletions(-)
create mode 100644 src/box/ck_constraint.c
create mode 100644 src/box/ck_constraint.h
--
2.21.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] [PATCH v4 1/4] schema: add new system space for CHECK constraints
2019-05-16 13:56 [tarantool-patches] [PATCH v4 0/4] box: run checks on insertions in LUA spaces Kirill Shcherbatov
@ 2019-05-16 13:56 ` Kirill Shcherbatov
2019-05-19 16:01 ` [tarantool-patches] " Vladislav Shpilevoy
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 2/4] box: run check constraint tests on space alter Kirill Shcherbatov
` (2 subsequent siblings)
3 siblings, 1 reply; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-16 13:56 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
This patch introduces a new system space to persist check
constraints. Format of the space:
_ck_constraint (space id = 357)
[<constraint name> STR, <space id> UINT,
<is_deferred>BOOL, <code>STR, <language>STR]
CK constraint is local to space, so every pair <CK name, space id>
is unique (and it is PK in _ck_constraint space).
After insertion into this space, a new instance describing check
constraint is created. Check constraint holds AST of expression
representing it.
While space features check constraints, it isn't allowed to
be dropped. The :drop() space method firstly deletes all check
constraints and then removes entry from _space.
Because space alter, index alter and space truncate operations
cause space recreation, introduced RebuildCkConstrains object
that compiles new ck constraint objects, replaces and removes
existent instances atomically(when some compilation fails,
nothing is changed). In fact, in scope of this patch we don't
really need to recreate ck_constraint object in such situations
(patch space_def pointer in AST like we did it before is enough
now, but we are going to recompile VDBE that represents ck
constraint in future that can fail).
The main motivation for these changes is the ability to support
ADD CHECK CONSTRAINT operation in the future. CK constraints are
are easier to manage as self-sustained objects: such change is
managed with atomic insertion(unlike the current architecture).
Disabled xfer optimization when some space have ck constraints
because in the following patches this xfer optimisation becomes
impossible. No reason to rewrite this code.
Needed for #3691
---
src/box/CMakeLists.txt | 1 +
src/box/alter.cc | 259 ++++++++++++++++++++++++--
src/box/alter.h | 1 +
src/box/bootstrap.snap | Bin 4374 -> 4428 bytes
src/box/ck_constraint.c | 117 ++++++++++++
src/box/ck_constraint.h | 174 +++++++++++++++++
src/box/errcode.h | 3 +-
src/box/lua/schema.lua | 4 +
src/box/lua/space.cc | 2 +
src/box/lua/upgrade.lua | 43 +++++
src/box/schema.cc | 8 +
src/box/schema_def.h | 11 ++
src/box/space.c | 2 +
src/box/space.h | 5 +
src/box/space_def.c | 98 +---------
src/box/space_def.h | 2 -
src/box/sql.c | 86 +--------
src/box/sql.h | 36 ----
src/box/sql/build.c | 211 +++++++++++++++++----
src/box/sql/insert.c | 55 +++---
src/box/sql/parse.y | 2 +-
src/box/sql/parse_def.h | 24 +++
src/box/sql/select.c | 11 +-
src/box/sql/sqlInt.h | 2 +-
src/box/sql/tokenize.c | 1 -
test/app-tap/tarantoolctl.test.lua | 4 +-
test/box-py/bootstrap.result | 7 +-
test/box/access.result | 3 +
test/box/access.test.lua | 1 +
test/box/access_misc.result | 3 +
test/box/access_sysview.result | 6 +-
test/box/alter.result | 6 +-
test/box/misc.result | 1 +
test/sql-tap/check.test.lua | 32 ++--
test/sql-tap/fkey2.test.lua | 4 +-
test/sql-tap/sql-errors.test.lua | 2 +-
test/sql-tap/table.test.lua | 4 +-
test/sql/checks.result | 215 ++++++++++++++++-----
test/sql/checks.test.lua | 98 ++++++----
test/sql/errinj.result | 134 +++++++++++++
test/sql/errinj.test.lua | 45 +++++
test/sql/gh-2981-check-autoinc.result | 8 +-
test/sql/types.result | 2 +-
test/sql/upgrade.result | 19 ++
test/sql/upgrade.test.lua | 5 +
test/wal_off/alter.result | 2 +-
46 files changed, 1339 insertions(+), 420 deletions(-)
create mode 100644 src/box/ck_constraint.c
create mode 100644 src/box/ck_constraint.h
diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 0864c3433..481842a39 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -97,6 +97,7 @@ add_library(box STATIC
space.c
space_def.c
sequence.c
+ ck_constraint.c
fk_constraint.c
func.c
func_def.c
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 9279426d2..e65c49d14 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -29,6 +29,7 @@
* SUCH DAMAGE.
*/
#include "alter.h"
+#include "ck_constraint.h"
#include "column_mask.h"
#include "schema.h"
#include "user.h"
@@ -529,17 +530,6 @@ space_def_new_from_tuple(struct tuple *tuple, uint32_t errcode,
engine_name, engine_name_len, &opts, fields,
field_count);
auto def_guard = make_scoped_guard([=] { space_def_delete(def); });
- if (def->opts.checks != NULL &&
- sql_checks_resolve_space_def_reference(def->opts.checks,
- def) != 0) {
- box_error_t *err = box_error_last();
- if (box_error_code(err) != ENOMEM) {
- tnt_raise(ClientError, errcode, def->name,
- box_error_message(err));
- } else {
- diag_raise();
- }
- }
struct engine *engine = engine_find_xc(def->engine_name);
engine_check_space_def_xc(engine, def);
def_guard.is_active = false;
@@ -1380,6 +1370,71 @@ UpdateSchemaVersion::alter(struct alter_space *alter)
++schema_version;
}
+/**
+ * As ck_constraint object depends on space_def we must rebuild
+ * all ck constraints on space alter.
+ *
+ * To perform it transactionally, we create a list of a new ck
+ * constraints objects in ::prepare method that is fault-tolerant.
+ * Finally in ::alter or ::rollback methods we only swap thouse
+ * lists securely.
+ */
+class RebuildCkConstraints: public AlterSpaceOp
+{
+public:
+ RebuildCkConstraints(struct alter_space *alter) : AlterSpaceOp(alter),
+ ck_constraint(RLIST_HEAD_INITIALIZER(ck_constraint)) {}
+ struct rlist ck_constraint;
+ virtual void prepare(struct alter_space *alter);
+ virtual void alter(struct alter_space *alter);
+ virtual void rollback(struct alter_space *alter);
+ virtual ~RebuildCkConstraints();
+};
+
+void
+RebuildCkConstraints::prepare(struct alter_space *alter)
+{
+ struct ck_constraint *old_ck_constraint;
+ rlist_foreach_entry(old_ck_constraint, &alter->old_space->ck_constraint,
+ link) {
+ struct ck_constraint *new_ck_constraint =
+ ck_constraint_new(old_ck_constraint->def,
+ alter->new_space->def);
+ if (new_ck_constraint == NULL)
+ diag_raise();
+ rlist_add_entry(&ck_constraint, new_ck_constraint, link);
+ }
+}
+
+void
+RebuildCkConstraints::alter(struct alter_space *alter)
+{
+ rlist_swap(&alter->new_space->ck_constraint, &ck_constraint);
+ rlist_swap(&ck_constraint, &alter->old_space->ck_constraint);
+}
+
+void
+RebuildCkConstraints::rollback(struct alter_space *alter)
+{
+ rlist_swap(&alter->old_space->ck_constraint, &ck_constraint);
+ rlist_swap(&ck_constraint, &alter->new_space->ck_constraint);
+}
+
+RebuildCkConstraints::~RebuildCkConstraints()
+{
+ struct ck_constraint *old_ck_constraint, *tmp;
+ rlist_foreach_entry_safe(old_ck_constraint, &ck_constraint, link, tmp) {
+ /**
+ * Ck constraint definition is now managed by
+ * other Ck constraint object. Prevent it's
+ * destruction as a part of ck_constraint_delete
+ * call.
+ */
+ old_ck_constraint->def = NULL;
+ ck_constraint_delete(old_ck_constraint);
+ }
+}
+
/* }}} */
/**
@@ -1745,6 +1800,11 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
space_name(old_space),
"the space has foreign key constraints");
}
+ if (!rlist_empty(&old_space->ck_constraint)) {
+ tnt_raise(ClientError, ER_DROP_SPACE,
+ space_name(old_space),
+ "the space has check constraints");
+ }
/**
* The space must be deleted from the space
* cache right away to achieve linearisable
@@ -1842,6 +1902,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
def->field_count);
(void) new CheckSpaceFormat(alter);
(void) new ModifySpace(alter, def);
+ (void) new RebuildCkConstraints(alter);
def_guard.is_active = false;
/* Create MoveIndex ops for all space indexes. */
alter_space_move_indexes(alter, 0, old_space->index_id_max + 1);
@@ -2085,6 +2146,7 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
*/
alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1);
/* Add an op to update schema_version on commit. */
+ (void) new RebuildCkConstraints(alter);
(void) new UpdateSchemaVersion(alter);
alter_space_do(txn, alter);
scoped_guard.is_active = false;
@@ -2152,6 +2214,7 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
(void) new TruncateIndex(alter, old_index->def->iid);
}
+ (void) new RebuildCkConstraints(alter);
alter_space_do(txn, alter);
scoped_guard.is_active = false;
}
@@ -4035,6 +4098,176 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
}
}
+/** Create an instance of check constraint definition by tuple. */
+static struct ck_constraint_def *
+ck_constraint_def_new_from_tuple(struct tuple *tuple)
+{
+ uint32_t name_len;
+ const char *name =
+ tuple_field_str_xc(tuple, BOX_CK_CONSTRAINT_FIELD_NAME,
+ &name_len);
+ if (name_len > BOX_NAME_MAX) {
+ tnt_raise(ClientError, ER_CREATE_CK_CONSTRAINT,
+ tt_cstr(name, BOX_INVALID_NAME_MAX),
+ "check constraint name is too long");
+ }
+ identifier_check_xc(name, name_len);
+ const char *language_str =
+ tuple_field_cstr_xc(tuple, BOX_CK_CONSTRAINT_FIELD_LANGUAGE);
+ enum ck_constraint_language language =
+ STR2ENUM(ck_constraint_language, language_str);
+ if (language == ck_constraint_language_MAX) {
+ tnt_raise(ClientError, ER_FUNCTION_LANGUAGE, language_str,
+ tt_cstr(name, name_len));
+ }
+ uint32_t expr_str_len;
+ const char *expr_str =
+ tuple_field_str_xc(tuple, BOX_CK_CONSTRAINT_FIELD_CODE,
+ &expr_str_len);
+ uint32_t name_offset, expr_str_offset;
+ uint32_t ck_def_sz =
+ ck_constraint_def_sizeof(name_len, expr_str_len, &name_offset,
+ &expr_str_offset);
+ struct ck_constraint_def *ck_def =
+ (struct ck_constraint_def *)malloc(ck_def_sz);
+ if (ck_def == NULL)
+ tnt_raise(OutOfMemory, ck_def_sz, "malloc", "ck_def");
+
+ ck_def->name = (char *)ck_def + name_offset;
+ ck_def->expr_str = (char *)ck_def + expr_str_offset;
+ ck_def->language = language;
+ memcpy(ck_def->expr_str, expr_str, expr_str_len);
+ ck_def->expr_str[expr_str_len] = '\0';
+ memcpy(ck_def->name, name, name_len);
+ ck_def->name[name_len] = '\0';
+
+ return ck_def;
+}
+
+/** Trigger invoked on rollback in the _ck_constraint space. */
+static void
+on_replace_ck_constraint_rollback(struct trigger *trigger, void *event)
+{
+ struct txn_stmt *stmt = txn_last_stmt((struct txn *) event);
+ struct ck_constraint *ck = (struct ck_constraint *)trigger->data;
+ struct space *space = NULL;
+ if (ck != NULL)
+ space = space_by_id(ck->space_id);
+ if (stmt->old_tuple != NULL && stmt->new_tuple == NULL) {
+ /* Rollback DELETE check constraint. */
+ assert(ck != NULL);
+ assert(space != NULL);
+ assert(space_ck_constraint_by_name(space,
+ ck->def->name, strlen(ck->def->name)) == NULL);
+ rlist_add_entry(&space->ck_constraint, ck, link);
+ } else if (stmt->new_tuple != NULL && stmt->old_tuple == NULL) {
+ /* Rollback INSERT check constraint. */
+ assert(space != NULL);
+ assert(space_ck_constraint_by_name(space,
+ ck->def->name, strlen(ck->def->name)) != NULL);
+ rlist_del_entry(ck, link);
+ ck_constraint_delete(ck);
+ } else {
+ /* Rollback REPLACE check constraint. */
+ assert(space != NULL);
+ const char *name = ck->def->name;
+ struct ck_constraint *new_ck =
+ space_ck_constraint_by_name(space, name, strlen(name));
+ assert(new_ck != NULL);
+ rlist_del_entry(new_ck, link);
+ rlist_add_entry(&space->ck_constraint, ck, link);
+ ck_constraint_delete(new_ck);
+ }
+}
+
+/**
+ * Trigger invoked on commit in the _ck_constraint space.
+ * Drop useless old check constraint object if exists.
+ */
+static void
+on_replace_ck_constraint_commit(struct trigger *trigger, void *event)
+{
+ struct txn_stmt *stmt = txn_last_stmt((struct txn *) event);
+ struct ck_constraint *ck = (struct ck_constraint *)trigger->data;
+ if (stmt->old_tuple != NULL)
+ ck_constraint_delete(ck);
+}
+
+/** A trigger invoked on replace in the _ck_constraint space. */
+static void
+on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event)
+{
+ struct txn *txn = (struct txn *) event;
+ txn_check_singlestatement_xc(txn, "Space _ck_constraint");
+ struct txn_stmt *stmt = txn_current_stmt(txn);
+ struct tuple *old_tuple = stmt->old_tuple;
+ struct tuple *new_tuple = stmt->new_tuple;
+ uint32_t space_id =
+ tuple_field_u32_xc(old_tuple != NULL ? old_tuple : new_tuple,
+ BOX_CK_CONSTRAINT_FIELD_SPACE_ID);
+ struct space *space = space_cache_find_xc(space_id);
+ struct trigger *on_rollback =
+ txn_alter_trigger_new(on_replace_ck_constraint_rollback, NULL);
+ struct trigger *on_commit =
+ txn_alter_trigger_new(on_replace_ck_constraint_commit, NULL);
+
+ if (new_tuple != NULL) {
+ bool is_deferred =
+ tuple_field_bool_xc(new_tuple,
+ BOX_CK_CONSTRAINT_FIELD_DEFERRED);
+ if (is_deferred) {
+ tnt_raise(ClientError, ER_UNSUPPORTED, "Tarantool",
+ "deferred ck constraints");
+ }
+ /* Create or replace check constraint. */
+ struct ck_constraint_def *ck_def =
+ ck_constraint_def_new_from_tuple(new_tuple);
+ auto ck_guard = make_scoped_guard([=] { free(ck_def); });
+ /*
+ * FIXME: Ck constraint creation on non-empty
+ * space must be implemented as preparatory
+ * step for ALTER SPACE ADD CONSTRAINT feature.
+ */
+ struct index *pk = space_index(space, 0);
+ if (pk != NULL && index_size(pk) > 0) {
+ tnt_raise(ClientError, ER_CREATE_CK_CONSTRAINT,
+ ck_def->name,
+ "referencing space must be empty");
+ }
+ struct ck_constraint *new_ck_constraint =
+ ck_constraint_new(ck_def, space->def);
+ if (new_ck_constraint == NULL)
+ diag_raise();
+ ck_guard.is_active = false;
+ const char *name = new_ck_constraint->def->name;
+ struct ck_constraint *old_ck_constraint =
+ space_ck_constraint_by_name(space, name, strlen(name));
+ if (old_ck_constraint != NULL)
+ rlist_del_entry(old_ck_constraint, link);
+ rlist_add_entry(&space->ck_constraint, new_ck_constraint, link);
+ on_commit->data = old_tuple == NULL ? new_ck_constraint :
+ old_ck_constraint;
+ on_rollback->data = on_commit->data;
+ } else {
+ assert(new_tuple == NULL && old_tuple != NULL);
+ /* Drop check constraint. */
+ uint32_t name_len;
+ const char *name =
+ tuple_field_str_xc(old_tuple,
+ BOX_CK_CONSTRAINT_FIELD_NAME,
+ &name_len);
+ struct ck_constraint *old_ck_constraint =
+ space_ck_constraint_by_name(space, name, name_len);
+ assert(old_ck_constraint != NULL);
+ rlist_del_entry(old_ck_constraint, link);
+ on_commit->data = old_ck_constraint;
+ on_rollback->data = old_ck_constraint;
+ }
+
+ txn_on_rollback(txn, on_rollback);
+ txn_on_commit(txn, on_commit);
+}
+
struct trigger alter_space_on_replace_space = {
RLIST_LINK_INITIALIZER, on_replace_dd_space, NULL, NULL
};
@@ -4103,4 +4336,8 @@ struct trigger on_replace_fk_constraint = {
RLIST_LINK_INITIALIZER, on_replace_dd_fk_constraint, NULL, NULL
};
+struct trigger on_replace_ck_constraint = {
+ RLIST_LINK_INITIALIZER, on_replace_dd_ck_constraint, NULL, NULL
+};
+
/* vim: set foldmethod=marker */
diff --git a/src/box/alter.h b/src/box/alter.h
index 4108fa47c..b9ba7b846 100644
--- a/src/box/alter.h
+++ b/src/box/alter.h
@@ -46,6 +46,7 @@ extern struct trigger on_replace_sequence_data;
extern struct trigger on_replace_space_sequence;
extern struct trigger on_replace_trigger;
extern struct trigger on_replace_fk_constraint;
+extern struct trigger on_replace_ck_constraint;
extern struct trigger on_stmt_begin_space;
extern struct trigger on_stmt_begin_index;
extern struct trigger on_stmt_begin_truncate;
diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap
diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c
new file mode 100644
index 000000000..db012837b
--- /dev/null
+++ b/src/box/ck_constraint.c
@@ -0,0 +1,117 @@
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include "box/session.h"
+#include "ck_constraint.h"
+#include "errcode.h"
+#include "sql.h"
+#include "sql/sqlInt.h"
+
+const char *ck_constraint_language_strs[] = {"SQL"};
+
+/**
+ * Resolve space_def references for check constraint via AST
+ * tree traversal.
+ * @param ck_constraint Check constraint object to update.
+ * @param space_def Space definition to use.
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+static int
+ck_constraint_resolve_field_names(struct Expr *expr,
+ struct space_def *space_def)
+{
+ struct Parse parser;
+ sql_parser_create(&parser, sql_get(), default_flags);
+ parser.parse_only = true;
+ sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL);
+ int rc = parser.is_aborted ? -1 : 0;
+ sql_parser_destroy(&parser);
+ return rc;
+}
+
+struct ck_constraint *
+ck_constraint_new(struct ck_constraint_def *ck_constraint_def,
+ struct space_def *space_def)
+{
+ if (space_def->field_count == 0) {
+ diag_set(ClientError, ER_UNSUPPORTED, "Tarantool",
+ "CK constraint for space without format");
+ return NULL;
+ }
+ struct ck_constraint *ck_constraint = malloc(sizeof(*ck_constraint));
+ if (ck_constraint == NULL) {
+ diag_set(OutOfMemory, sizeof(*ck_constraint), "malloc",
+ "ck_constraint");
+ return NULL;
+ }
+ ck_constraint->def = NULL;
+ ck_constraint->space_id = space_def->id;
+ rlist_create(&ck_constraint->link);
+ ck_constraint->expr =
+ sql_expr_compile(sql_get(), ck_constraint_def->expr_str,
+ strlen(ck_constraint_def->expr_str));
+ if (ck_constraint->expr == NULL ||
+ ck_constraint_resolve_field_names(ck_constraint->expr,
+ space_def) != 0) {
+ diag_set(ClientError, ER_CREATE_CK_CONSTRAINT,
+ ck_constraint_def->name,
+ box_error_message(box_error_last()));
+ goto error;
+ }
+
+ ck_constraint->def = ck_constraint_def;
+ return ck_constraint;
+error:
+ ck_constraint_delete(ck_constraint);
+ return NULL;
+}
+
+void
+ck_constraint_delete(struct ck_constraint *ck_constraint)
+{
+ sql_expr_delete(sql_get(), ck_constraint->expr, false);
+ free(ck_constraint->def);
+ TRASH(ck_constraint);
+ free(ck_constraint);
+}
+
+struct ck_constraint *
+space_ck_constraint_by_name(struct space *space, const char *name,
+ uint32_t name_len)
+{
+ struct ck_constraint *ck_constraint = NULL;
+ rlist_foreach_entry(ck_constraint, &space->ck_constraint, link) {
+ if (strlen(ck_constraint->def->name) == name_len &&
+ memcmp(ck_constraint->def->name, name, name_len) == 0)
+ return ck_constraint;
+ }
+ return NULL;
+}
diff --git a/src/box/ck_constraint.h b/src/box/ck_constraint.h
new file mode 100644
index 000000000..9c72d5977
--- /dev/null
+++ b/src/box/ck_constraint.h
@@ -0,0 +1,174 @@
+#ifndef INCLUDES_BOX_CK_CONSTRAINT_H
+#define INCLUDES_BOX_CK_CONSTRAINT_H
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <stdint.h>
+#include "small/rlist.h"
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+struct space;
+struct space_def;
+struct Expr;
+
+/** The supported language of the ck constraint. */
+enum ck_constraint_language {
+ CK_CONSTRAINT_LANGUAGE_SQL,
+ ck_constraint_language_MAX,
+};
+
+/** The supported languages strings of the ck constraint. */
+extern const char *ck_constraint_language_strs[];
+
+/**
+ * Check constraint definition.
+ * See ck_constraint_def_sizeof() definition for implementation
+ * details and memory layout.
+ */
+struct ck_constraint_def {
+ /**
+ * The 0-terminated string, a name of the check
+ * constraint. Must be unique for a given space.
+ */
+ char *name;
+ /**
+ * The 0-terminated string that defines check constraint
+ * expression.
+ *
+ * For instance: "field1 + field2 > 2 * 3".
+ */
+ char *expr_str;
+ /** The language of ck constraint. */
+ enum ck_constraint_language language;
+};
+
+/**
+ * Structure representing check constraint.
+ * See ck_constraint_new() definition.
+ */
+struct ck_constraint {
+ /** The check constraint definition. */
+ 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 sql_resolve_self_reference for
+ * space with space[ck_constraint::space_id] definition.
+ */
+ struct Expr *expr;
+ /**
+ * The id of the space this check constraint is
+ * built for.
+ */
+ uint32_t space_id;
+ /**
+ * Organize check constraint structs into linked list
+ * with space::ck_constraint.
+ */
+ struct rlist link;
+};
+
+/**
+ * Calculate check constraint definition memory size and fields
+ * offsets for given arguments.
+ *
+ * Alongside with struct ck_constraint_def itself, we reserve
+ * memory for string containing its name and expression string.
+ *
+ * Memory layout:
+ * +-----------------------------+ <- Allocated memory starts here
+ * | struct ck_constraint_def |
+ * |-----------------------------|
+ * | name + \0 |
+ * |-----------------------------|
+ * | expr_str + \0 |
+ * +-----------------------------+
+ *
+ * @param name_len The length of the name.
+ * @param expr_str_len The length of the expr_str.
+ * @param[out] name_offset The offset of the name string.
+ * @param[out] expr_str_offset The offset of the expr_str string.
+ */
+static inline uint32_t
+ck_constraint_def_sizeof(uint32_t name_len, uint32_t expr_str_len,
+ uint32_t *name_offset, uint32_t *expr_str_offset)
+{
+ *name_offset = sizeof(struct ck_constraint_def);
+ *expr_str_offset = *name_offset + name_len + 1;
+ return *expr_str_offset + expr_str_len + 1;
+}
+
+/**
+ * Create a new check constraint object by given check constraint
+ * definition and definition of the space this constraint is
+ * related to.
+ *
+ * @param ck_constraint_def The check constraint definition object
+ * to use. Expected to be allocated with
+ * malloc. Ck constraint object manages
+ * this allocation in case of successful
+ * creation.
+ * @param space_def The space definition of the space this check
+ * constraint must be constructed for.
+ * @retval not NULL Check constraint object pointer on success.
+ * @retval NULL Otherwise.
+*/
+struct ck_constraint *
+ck_constraint_new(struct ck_constraint_def *ck_constraint_def,
+ struct space_def *space_def);
+
+/**
+ * Destroy check constraint memory, release acquired resources.
+ * @param ck_constraint The check constraint object to destroy.
+ */
+void
+ck_constraint_delete(struct ck_constraint *ck_constraint);
+
+/**
+ * Find check constraint object in space by given name and
+ * name_len.
+ * @param space The space to lookup check constraint.
+ * @param name The check constraint name.
+ * @param name_len The length of the name.
+ * @retval not NULL Check constrain if exists, NULL otherwise.
+ */
+struct ck_constraint *
+space_ck_constraint_by_name(struct space *space, const char *name,
+ uint32_t name_len);
+
+#if defined(__cplusplus)
+} /* extern "C" { */
+#endif
+
+#endif /* INCLUDES_BOX_CK_CONSTRAINT_H */
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 9c15f3322..1f7c81693 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -245,8 +245,9 @@ struct errcode_record {
/*190 */_(ER_INT_LITERAL_MAX, "Integer literal %s%s exceeds the supported range %lld - %lld") \
/*191 */_(ER_SQL_PARSER_LIMIT, "%s %d exceeds the limit (%d)") \
/*192 */_(ER_INDEX_DEF_UNSUPPORTED, "%s are prohibited in an index definition") \
- /*193 */_(ER_CK_DEF_UNSUPPORTED, "%s are prohibited in a CHECK constraint definition") \
+ /*193 */_(ER_CK_DEF_UNSUPPORTED, "%s are prohibited in a ck constraint definition") \
/*194 */_(ER_MULTIKEY_INDEX_MISMATCH, "Field %s is used as multikey in one index and as single key in another") \
+ /*195 */_(ER_CREATE_CK_CONSTRAINT, "Failed to create check constraint '%s': %s") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index f31cf7f2c..e01f500e6 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -513,6 +513,7 @@ box.schema.space.drop = function(space_id, space_name, opts)
local _truncate = box.space[box.schema.TRUNCATE_ID]
local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID]
local _fk_constraint = box.space[box.schema.FK_CONSTRAINT_ID]
+ local _ck_constraint = box.space[box.schema.CK_CONSTRAINT_ID]
local sequence_tuple = _space_sequence:delete{space_id}
if sequence_tuple ~= nil and sequence_tuple[3] == true then
-- Delete automatically generated sequence.
@@ -524,6 +525,9 @@ box.schema.space.drop = function(space_id, space_name, opts)
for _, t in _fk_constraint.index.child_id:pairs({space_id}) do
_fk_constraint:delete({t.name, space_id})
end
+ for _, t in _ck_constraint.index.space_id:pairs({space_id}) do
+ _ck_constraint:delete({t.name, space_id})
+ end
local keys = _vindex:select(space_id)
for i = #keys, 1, -1 do
local v = keys[i]
diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index 100da0a79..ce4287bba 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -537,6 +537,8 @@ box_lua_space_init(struct lua_State *L)
lua_setfield(L, -2, "TRIGGER_ID");
lua_pushnumber(L, BOX_FK_CONSTRAINT_ID);
lua_setfield(L, -2, "FK_CONSTRAINT_ID");
+ lua_pushnumber(L, BOX_CK_CONSTRAINT_ID);
+ lua_setfield(L, -2, "CK_CONSTRAINT_ID");
lua_pushnumber(L, BOX_TRUNCATE_ID);
lua_setfield(L, -2, "TRUNCATE_ID");
lua_pushnumber(L, BOX_SEQUENCE_ID);
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 89d6e3d52..7f2656eb7 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -74,6 +74,7 @@ local function set_system_triggers(val)
box.space._schema:run_triggers(val)
box.space._cluster:run_triggers(val)
box.space._fk_constraint:run_triggers(val)
+ box.space._ck_constraint:run_triggers(val)
end
--------------------------------------------------------------------------------
@@ -94,6 +95,7 @@ local function erase()
truncate(box.space._schema)
truncate(box.space._cluster)
truncate(box.space._fk_constraint)
+ truncate(box.space._ck_constraint)
end
local function create_sysview(source_id, target_id)
@@ -737,6 +739,46 @@ local function upgrade_to_2_1_3()
end
end
+local function upgrade_to_2_2_0()
+ -- In previous Tarantool releases check constraints were
+ -- stored in space opts. Now we use separate space
+ -- _ck_constraint for this purpose. Perform legacy data
+ -- migration.
+ local MAP = setmap({})
+ local _space = box.space[box.schema.SPACE_ID]
+ local _index = box.space[box.schema.INDEX_ID]
+ local _ck_constraint = box.space[box.schema.CK_CONSTRAINT_ID]
+ log.info("create space _ck_constraint")
+ local format = {{name='name', type='string'},
+ {name='space_id', type='unsigned'},
+ {name='is_deferred', type='boolean'},
+ {name='code', type='str'}, {name='language', type='str'}}
+ _space:insert{_ck_constraint.id, ADMIN, '_ck_constraint', 'memtx', 0, MAP, format}
+
+ log.info("create index primary on _ck_constraint")
+ _index:insert{_ck_constraint.id, 0, 'primary', 'tree',
+ {unique = true}, {{0, 'string'}, {1, 'unsigned'}}}
+
+ log.info("create secondary index child_id on _ck_constraint")
+ _index:insert{_ck_constraint.id, 1, 'space_id', 'tree',
+ {unique = false}, {{1, 'unsigned'}}}
+ for _, space in _space:pairs() do
+ local flags = space.flags
+ if flags['checks'] ~= nil then
+ for i, check in pairs(flags['checks']) do
+ local expr_str = check.expr
+ local check_name = check.name or
+ "CK_CONSTRAINT_"..i.."_"..space.name
+ _ck_constraint:insert({check_name, space.id, false, expr_str, 'SQL'})
+ end
+ flags['checks'] = nil
+ _space:replace(box.tuple.new({space.id, space.owner, space.name,
+ space.engine, space.field_count,
+ flags, space.format}))
+ end
+ end
+end
+
local function get_version()
local version = box.space._schema:get{'version'}
if version == nil then
@@ -768,6 +810,7 @@ local function upgrade(options)
{version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true},
{version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true},
{version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true},
+ {version = mkversion(2, 2, 0), func = upgrade_to_2_2_0, auto = true},
}
for _, handler in ipairs(handlers) do
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 9a55c2f14..4b20f0032 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -460,6 +460,14 @@ schema_init()
sc_space_new(BOX_FK_CONSTRAINT_ID, "_fk_constraint", key_parts, 2,
&on_replace_fk_constraint, NULL);
+ /* _ck_сonstraint - check constraints. */
+ key_parts[0].fieldno = 0; /* constraint name */
+ key_parts[0].type = FIELD_TYPE_STRING;
+ key_parts[1].fieldno = 1; /* space id */
+ key_parts[1].type = FIELD_TYPE_UNSIGNED;
+ sc_space_new(BOX_CK_CONSTRAINT_ID, "_ck_constraint", key_parts, 2,
+ &on_replace_ck_constraint, NULL);
+
/*
* _vinyl_deferred_delete - blackhole that is needed
* for writing deferred DELETE statements generated by
diff --git a/src/box/schema_def.h b/src/box/schema_def.h
index eeeeb950b..6bd34ba34 100644
--- a/src/box/schema_def.h
+++ b/src/box/schema_def.h
@@ -108,6 +108,8 @@ enum {
BOX_SPACE_SEQUENCE_ID = 340,
/** Space id of _fk_constraint. */
BOX_FK_CONSTRAINT_ID = 356,
+ /** Space id of _ck_contraint. */
+ BOX_CK_CONSTRAINT_ID = 357,
/** End of the reserved range of system spaces. */
BOX_SYSTEM_ID_MAX = 511,
BOX_ID_NIL = 2147483647
@@ -238,6 +240,15 @@ enum {
BOX_FK_CONSTRAINT_FIELD_PARENT_COLS = 8,
};
+/** _ck_constraint fields. */
+enum {
+ BOX_CK_CONSTRAINT_FIELD_NAME = 0,
+ BOX_CK_CONSTRAINT_FIELD_SPACE_ID = 1,
+ BOX_CK_CONSTRAINT_FIELD_DEFERRED = 2,
+ BOX_CK_CONSTRAINT_FIELD_CODE = 3,
+ BOX_CK_CONSTRAINT_FIELD_LANGUAGE = 4,
+};
+
/*
* Different objects which can be subject to access
* control.
diff --git a/src/box/space.c b/src/box/space.c
index 243e7da2f..a42b3a64b 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -165,6 +165,7 @@ space_create(struct space *space, struct engine *engine,
space_fill_index_map(space);
rlist_create(&space->parent_fk_constraint);
rlist_create(&space->child_fk_constraint);
+ rlist_create(&space->ck_constraint);
return 0;
fail_free_indexes:
@@ -225,6 +226,7 @@ space_delete(struct space *space)
assert(space->sql_triggers == NULL);
assert(rlist_empty(&space->parent_fk_constraint));
assert(rlist_empty(&space->child_fk_constraint));
+ assert(rlist_empty(&space->ck_constraint));
space->vtab->destroy(space);
}
diff --git a/src/box/space.h b/src/box/space.h
index 13a220d13..e8529beb3 100644
--- a/src/box/space.h
+++ b/src/box/space.h
@@ -193,6 +193,11 @@ struct space {
* of index id.
*/
struct index **index;
+ /**
+ * List of check constraints linked with
+ * ck_constraint::link.
+ */
+ struct rlist ck_constraint;
/**
* Lists of foreign key constraints. In SQL terms child
* space is the "from" table i.e. the table that contains
diff --git a/src/box/space_def.c b/src/box/space_def.c
index d825def75..661131295 100644
--- a/src/box/space_def.c
+++ b/src/box/space_def.c
@@ -36,28 +36,12 @@
#include "msgpuck.h"
#include "tt_static.h"
-/**
- * Make checks from msgpack.
- * @param str pointer to array of maps
- * e.g. [{"expr": "x < y", "name": "ONE"}, ..].
- * @param len array items count.
- * @param[out] opt pointer to store parsing result.
- * @param errcode Code of error to set if something is wrong.
- * @param field_no Field number of an option in a parent element.
- * @retval 0 on success.
- * @retval not 0 on error. Also set diag message.
- */
-static int
-checks_array_decode(const char **str, uint32_t len, char *opt, uint32_t errcode,
- uint32_t field_no);
-
const struct space_opts space_opts_default = {
/* .group_id = */ 0,
/* .is_temporary = */ false,
/* .is_ephemeral = */ false,
/* .view = */ false,
/* .sql = */ NULL,
- /* .checks = */ NULL,
};
const struct opt_def space_opts_reg[] = {
@@ -65,8 +49,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,
- checks_array_decode),
+ OPT_DEF_LEGACY("checks"),
OPT_END,
};
@@ -114,16 +97,6 @@ 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) {
- 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);
- }
return 0;
}
@@ -301,74 +274,5 @@ void
space_opts_destroy(struct space_opts *opts)
{
free(opts->sql);
- sql_expr_list_delete(sql_get(), opts->checks);
TRASH(opts);
}
-
-static int
-checks_array_decode(const char **str, uint32_t len, char *opt, uint32_t errcode,
- uint32_t field_no)
-{
- char *errmsg = tt_static_buf();
- struct ExprList *checks = NULL;
- const char **map = str;
- struct sql *db = sql_get();
- for (uint32_t i = 0; i < len; i++) {
- checks = sql_expr_list_append(db, checks, NULL);
- if (checks == NULL) {
- diag_set(OutOfMemory, 0, "sql_expr_list_append",
- "checks");
- goto error;
- }
- const char *expr_name = NULL;
- const char *expr_str = NULL;
- uint32_t expr_name_len = 0;
- uint32_t expr_str_len = 0;
- uint32_t map_size = mp_decode_map(map);
- for (uint32_t j = 0; j < map_size; j++) {
- if (mp_typeof(**map) != MP_STR) {
- diag_set(ClientError, errcode, field_no,
- "key must be a string");
- goto error;
- }
- uint32_t key_len;
- const char *key = mp_decode_str(map, &key_len);
- if (mp_typeof(**map) != MP_STR) {
- snprintf(errmsg, TT_STATIC_BUF_LEN,
- "invalid MsgPack map field '%.*s' type",
- key_len, key);
- diag_set(ClientError, errcode, field_no, errmsg);
- goto error;
- }
- if (key_len == 4 && memcmp(key, "expr", key_len) == 0) {
- expr_str = mp_decode_str(map, &expr_str_len);
- } else if (key_len == 4 &&
- memcmp(key, "name", key_len) == 0) {
- expr_name = mp_decode_str(map, &expr_name_len);
- } else {
- snprintf(errmsg, TT_STATIC_BUF_LEN,
- "invalid MsgPack map field '%.*s'",
- key_len, key);
- diag_set(ClientError, errcode, field_no, errmsg);
- goto error;
- }
- }
- if (sql_check_list_item_init(checks, i, expr_name, expr_name_len,
- expr_str, expr_str_len) != 0) {
- box_error_t *err = box_error_last();
- if (box_error_code(err) != ENOMEM) {
- snprintf(errmsg, TT_STATIC_BUF_LEN,
- "invalid expression specified (%s)",
- box_error_message(err));
- diag_set(ClientError, errcode, field_no,
- errmsg);
- }
- goto error;
- }
- }
- *(struct ExprList **)opt = checks;
- return 0;
-error:
- sql_expr_list_delete(db, checks);
- return -1;
-}
diff --git a/src/box/space_def.h b/src/box/space_def.h
index c6639a8d8..71356e2a6 100644
--- a/src/box/space_def.h
+++ b/src/box/space_def.h
@@ -71,8 +71,6 @@ struct space_opts {
bool is_view;
/** SQL statement that produced this space. */
char *sql;
- /** SQL Checks expressions list. */
- struct ExprList *checks;
};
extern const struct space_opts space_opts_default;
diff --git a/src/box/sql.c b/src/box/sql.c
index 385676055..c5123fcc0 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1035,15 +1035,8 @@ sql_encode_table_opts(struct region *region, struct space_def *def,
bool is_error = false;
mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
set_encode_error, &is_error);
- int checks_cnt = 0;
- struct ExprList_item *a;
bool is_view = def->opts.is_view;
- struct ExprList *checks = def->opts.checks;
- if (checks != NULL) {
- checks_cnt = checks->nExpr;
- a = checks->a;
- }
- mpstream_encode_map(&stream, 2 * is_view + (checks_cnt > 0));
+ mpstream_encode_map(&stream, 2 * is_view);
if (is_view) {
assert(def->opts.sql != NULL);
@@ -1052,23 +1045,6 @@ sql_encode_table_opts(struct region *region, struct space_def *def,
mpstream_encode_str(&stream, "view");
mpstream_encode_bool(&stream, true);
}
- if (checks_cnt > 0) {
- mpstream_encode_str(&stream, "checks");
- mpstream_encode_array(&stream, checks_cnt);
- }
- for (int i = 0; i < checks_cnt && !is_error; ++i, ++a) {
- int items = (a->pExpr != NULL) + (a->zName != NULL);
- mpstream_encode_map(&stream, items);
- assert(a->pExpr != NULL);
- struct Expr *pExpr = a->pExpr;
- assert(pExpr->u.zToken != NULL);
- mpstream_encode_str(&stream, "expr");
- mpstream_encode_str(&stream, pExpr->u.zToken);
- if (a->zName != NULL) {
- mpstream_encode_str(&stream, "name");
- mpstream_encode_str(&stream, a->zName);
- }
- }
mpstream_flush(&stream);
if (is_error) {
diag_set(OutOfMemory, stream.pos - stream.buf,
@@ -1302,63 +1278,3 @@ sql_ephemeral_space_new(Parse *parser, const char *name)
return space;
}
-
-int
-sql_check_list_item_init(struct ExprList *expr_list, int column,
- const char *expr_name, uint32_t expr_name_len,
- const char *expr_str, uint32_t expr_str_len)
-{
- assert(column < expr_list->nExpr);
- struct ExprList_item *item = &expr_list->a[column];
- memset(item, 0, sizeof(*item));
- if (expr_name != NULL) {
- item->zName = sqlDbStrNDup(db, expr_name, expr_name_len);
- if (item->zName == NULL) {
- diag_set(OutOfMemory, expr_name_len, "sqlDbStrNDup",
- "item->zName");
- return -1;
- }
- }
- if (expr_str != NULL) {
- item->pExpr = sql_expr_compile(db, expr_str, expr_str_len);
- /* The item->zName would be released later. */
- if (item->pExpr == NULL)
- return -1;
- }
- return 0;
-}
-
-static int
-update_space_def_callback(Walker *walker, Expr *expr)
-{
- if (expr->op == TK_COLUMN && ExprHasProperty(expr, EP_Resolved))
- expr->space_def = walker->u.space_def;
- return WRC_Continue;
-}
-
-void
-sql_checks_update_space_def_reference(ExprList *expr_list,
- struct space_def *def)
-{
- assert(expr_list != NULL);
- Walker w;
- memset(&w, 0, sizeof(w));
- w.xExprCallback = update_space_def_callback;
- w.u.space_def = def;
- for (int i = 0; i < expr_list->nExpr; i++)
- sqlWalkExpr(&w, expr_list->a[i].pExpr);
-}
-
-int
-sql_checks_resolve_space_def_reference(ExprList *expr_list,
- struct space_def *def)
-{
- Parse parser;
- sql_parser_create(&parser, sql_get(), default_flags);
- parser.parse_only = true;
-
- sql_resolve_self_reference(&parser, def, NC_IsCheck, NULL, expr_list);
- int rc = parser.is_aborted ? -1 : 0;
- sql_parser_destroy(&parser);
- return rc;
-}
diff --git a/src/box/sql.h b/src/box/sql.h
index 15ef74b19..9c8a96a75 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -285,42 +285,6 @@ sql_resolve_self_reference(struct Parse *parser, struct space_def *def,
int type, struct Expr *expr,
struct ExprList *expr_list);
-/**
- * Initialize check_list_item.
- * @param expr_list ExprList with item.
- * @param column index.
- * @param expr_name expression name (optional).
- * @param expr_name_len expresson name length (optional).
- * @param expr_str expression to build string.
- * @param expr_str_len expression to build string length.
- * @retval 0 on success.
- * @retval -1 on error.
- */
-int
-sql_check_list_item_init(struct ExprList *expr_list, int column,
- const char *expr_name, uint32_t expr_name_len,
- const char *expr_str, uint32_t expr_str_len);
-
-/**
- * Resolve space_def references checks for expr_list.
- * @param expr_list to modify.
- * @param def to refer to.
- * @retval 0 on success.
- * @retval -1 on error.
- */
-int
-sql_checks_resolve_space_def_reference(struct ExprList *expr_list,
- struct space_def *def);
-
-/**
- * Update space_def references for expr_list.
- * @param expr_list to modify.
- * @param def to refer to.
- */
-void
-sql_checks_update_space_def_reference(struct ExprList *expr_list,
- struct space_def *def);
-
/**
* Initialize a new parser object.
* A number of service allocations are performed on the region,
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 6051a2529..f2bbbb9c4 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -43,10 +43,12 @@
* COMMIT
* ROLLBACK
*/
+#include <ctype.h>
#include "sqlInt.h"
#include "vdbeInt.h"
#include "tarantoolInt.h"
#include "box/box.h"
+#include "box/ck_constraint.h"
#include "box/fk_constraint.h"
#include "box/sequence.h"
#include "box/session.h"
@@ -638,40 +640,109 @@ primary_key_exit:
return;
}
+/**
+ * Prepare a 0-terminated string in the wptr memory buffer that
+ * does not contain a sequence of more than one whatespace
+ * character. Routine enforces ' ' (space) as whitespace
+ * delimiter.
+ * The wptr buffer is expected to have str_len + 1 bytes
+ * (this is the expected scenario where no extra whitespace
+ * characters preset in the source string).
+ * @param wptr The destination memory buffer of size
+ * @a str_len + 1.
+ * @param str The source string to be copied.
+ * @param str_len The source string @a str length.
+ */
+static void
+trim_space_snprintf(char *wptr, const char *str, uint32_t str_len)
+{
+ const char *str_end = str + str_len;
+ bool is_prev_chr_space = false;
+ while (str < str_end) {
+ if (isspace((unsigned char)*str)) {
+ if (!is_prev_chr_space)
+ *wptr++ = ' ';
+ is_prev_chr_space = true;
+ str++;
+ continue;
+ }
+ is_prev_chr_space = false;
+ *wptr++ = *str++;
+ }
+ *wptr = '\0';
+}
+
void
-sql_add_check_constraint(struct Parse *parser)
+sql_create_check_contraint(struct Parse *parser)
{
- struct create_ck_def *ck_def = &parser->create_ck_def;
+ struct create_ck_def *create_ck_def = &parser->create_ck_def;
+ struct ExprSpan *expr_span = create_ck_def->expr;
+ sql_expr_delete(parser->db, expr_span->pExpr, false);
+
struct alter_entity_def *alter_def =
(struct alter_entity_def *) &parser->create_ck_def;
assert(alter_def->entity_type == ENTITY_TYPE_CK);
(void) alter_def;
- struct Expr *expr = ck_def->expr->pExpr;
struct space *space = parser->create_table_def.new_space;
- if (space != NULL) {
- expr->u.zToken =
- sqlDbStrNDup(parser->db,
- (char *) ck_def->expr->zStart,
- (int) (ck_def->expr->zEnd -
- ck_def->expr->zStart));
- if (expr->u.zToken == NULL)
- goto release_expr;
- space->def->opts.checks =
- sql_expr_list_append(parser->db,
- space->def->opts.checks, expr);
- if (space->def->opts.checks == NULL) {
- sqlDbFree(parser->db, expr->u.zToken);
- goto release_expr;
- }
- struct create_entity_def *entity_def = &ck_def->base.base;
- if (entity_def->name.n > 0) {
- sqlExprListSetName(parser, space->def->opts.checks,
- &entity_def->name, 1);
+ assert(space != NULL);
+
+ /* Prepare payload for ck constraint definition. */
+ struct region *region = &parser->region;
+ struct Token *name_token = &create_ck_def->base.base.name;
+ const char *name;
+ if (name_token->n > 0) {
+ name = sql_normalized_name_region_new(region, name_token->z,
+ name_token->n);
+ if (name == NULL) {
+ parser->is_aborted = true;
+ return;
}
} else {
-release_expr:
- sql_expr_delete(parser->db, expr, false);
+ uint32_t ck_idx = ++parser->create_table_def.check_count;
+ name = tt_sprintf("CK_CONSTRAINT_%d_%s", ck_idx,
+ space->def->name);
+ }
+ size_t name_len = strlen(name);
+
+ uint32_t expr_str_len = (uint32_t)(create_ck_def->expr->zEnd -
+ create_ck_def->expr->zStart);
+ const char *expr_str = create_ck_def->expr->zStart;
+
+ /*
+ * Allocate memory for ck constraint parse structure and
+ * ck constraint definition as a single memory chunk on
+ * region:
+ *
+ * [ck_parse][ck_def[name][expr_str]]
+ * |_____^ |___^ ^
+ * |_________|
+ */
+ uint32_t name_offset, expr_str_offset;
+ uint32_t ck_def_sz =
+ ck_constraint_def_sizeof(name_len, expr_str_len, &name_offset,
+ &expr_str_offset);
+ struct ck_constraint_parse *ck_parse =
+ region_alloc(region, sizeof(*ck_parse) + ck_def_sz);
+ if (ck_parse == NULL) {
+ diag_set(OutOfMemory, sizeof(*ck_parse) + ck_def_sz, "region",
+ "ck_parse");
+ parser->is_aborted = true;
+ return;
}
+ struct ck_constraint_def *ck_def =
+ (struct ck_constraint_def *)((char *)ck_parse +
+ sizeof(*ck_parse));
+ ck_parse->ck_def = ck_def;
+ rlist_create(&ck_parse->link);
+
+ ck_def->name = (char *)ck_def + name_offset;
+ ck_def->expr_str = (char *)ck_def + expr_str_offset;
+ ck_def->language = CK_CONSTRAINT_LANGUAGE_SQL;
+ trim_space_snprintf(ck_def->expr_str, expr_str, expr_str_len);
+ memcpy(ck_def->name, name, name_len);
+ ck_def->name[name_len] = '\0';
+
+ rlist_add_entry(&parser->create_table_def.new_check, ck_parse, link);
}
/*
@@ -939,6 +1010,40 @@ emitNewSysSequenceRecord(Parse *pParse, int reg_seq_id, const char *seq_name)
return first_col;
}
+/**
+ * Generate opcodes to serialize check constraint definition into
+ * MsgPack and insert produced tuple into _ck_constraint space.
+ * @param parser Parsing context.
+ * @param ck_def Check constraint definition to be serialized.
+ * @param reg_space_id The VDBE register containing space id.
+*/
+static void
+vdbe_emit_ck_constraint_create(struct Parse *parser,
+ const struct ck_constraint_def *ck_def,
+ uint32_t reg_space_id)
+{
+ struct sql *db = parser->db;
+ struct Vdbe *v = sqlGetVdbe(parser);
+ assert(v != NULL);
+ int ck_constraint_reg = sqlGetTempRange(parser, 6);
+ sqlVdbeAddOp4(v, OP_String8, 0, ck_constraint_reg, 0,
+ sqlDbStrDup(db, ck_def->name), P4_DYNAMIC);
+ sqlVdbeAddOp2(v, OP_SCopy, reg_space_id, ck_constraint_reg + 1);
+ sqlVdbeAddOp2(v, OP_Bool, false, ck_constraint_reg + 2);
+ sqlVdbeAddOp4(v, OP_String8, 0, ck_constraint_reg + 3, 0,
+ sqlDbStrDup(db, ck_def->expr_str), P4_DYNAMIC);
+ sqlVdbeAddOp4(v, OP_String8, 0, ck_constraint_reg + 4, 0,
+ ck_constraint_language_strs[ck_def->language], P4_STATIC);
+ sqlVdbeAddOp3(v, OP_MakeRecord, ck_constraint_reg, 5,
+ ck_constraint_reg + 5);
+ sqlVdbeAddOp3(v, OP_SInsert, BOX_CK_CONSTRAINT_ID, 0,
+ ck_constraint_reg + 5);
+ save_record(parser, BOX_CK_CONSTRAINT_ID, ck_constraint_reg, 2,
+ v->nOp - 1);
+ VdbeComment((v, "Create CK constraint %s", ck_def->name));
+ sqlReleaseTempRange(parser, ck_constraint_reg, 5);
+}
+
int
emitNewSysSpaceSequenceRecord(Parse *pParse, int space_id, const char reg_seq_id)
{
@@ -1108,20 +1213,18 @@ resolve_link(struct Parse *parse_context, const struct space_def *def,
void
sqlEndTable(struct Parse *pParse)
{
- sql *db = pParse->db; /* The database connection */
-
- assert(!db->mallocFailed);
+ assert(!pParse->db->mallocFailed);
struct space *new_space = pParse->create_table_def.new_space;
if (new_space == NULL)
return;
- assert(!db->init.busy);
+ assert(!pParse->db->init.busy);
assert(!new_space->def->opts.is_view);
if (sql_space_primary_key(new_space) == NULL) {
diag_set(ClientError, ER_CREATE_SPACE, new_space->def->name,
"PRIMARY KEY missing");
pParse->is_aborted = true;
- goto cleanup;
+ return;
}
/*
@@ -1211,9 +1314,12 @@ sqlEndTable(struct Parse *pParse)
fk_def->child_id = reg_space_id;
vdbe_emit_fk_constraint_create(pParse, fk_def);
}
-cleanup:
- sql_expr_list_delete(db, new_space->def->opts.checks);
- new_space->def->opts.checks = NULL;
+ struct ck_constraint_parse *ck_parse;
+ rlist_foreach_entry(ck_parse, &pParse->create_table_def.new_check,
+ link) {
+ vdbe_emit_ck_constraint_create(pParse, ck_parse->ck_def,
+ reg_space_id);
+ }
}
void
@@ -1411,6 +1517,37 @@ vdbe_emit_fk_constraint_drop(struct Parse *parse_context, char *constraint_name,
sqlReleaseTempRange(parse_context, key_reg, 3);
}
+/**
+ * Generate VDBE program to remove entry from _ck_constraint space.
+ *
+ * @param parser Parsing context.
+ * @param ck_name Name of CK constraint to be dropped.
+ * @param child_id Id of table which constraint belongs to.
+ */
+static void
+vdbe_emit_ck_constraint_drop(struct Parse *parser, const char *ck_name,
+ uint32_t space_id)
+{
+ struct Vdbe *v = sqlGetVdbe(parser);
+ struct sql *db = v->db;
+ assert(v != NULL);
+ int key_reg = sqlGetTempRange(parser, 3);
+ sqlVdbeAddOp4(v, OP_String8, 0, key_reg, 0, sqlDbStrDup(db, ck_name),
+ P4_DYNAMIC);
+ sqlVdbeAddOp2(v, OP_Integer, space_id, key_reg + 1);
+ const char *error_msg =
+ tt_sprintf(tnt_errcode_desc(ER_NO_SUCH_CONSTRAINT), ck_name);
+ if (vdbe_emit_halt_with_presence_test(parser, BOX_CK_CONSTRAINT_ID, 0,
+ key_reg, 2, ER_NO_SUCH_CONSTRAINT,
+ error_msg, false,
+ OP_Found) != 0)
+ return;
+ sqlVdbeAddOp3(v, OP_MakeRecord, key_reg, 2, key_reg + 2);
+ sqlVdbeAddOp2(v, OP_SDelete, BOX_CK_CONSTRAINT_ID, key_reg + 2);
+ VdbeComment((v, "Delete CK constraint %s", ck_name));
+ sqlReleaseTempRange(parser, key_reg, 3);
+}
+
/**
* Generate code to drop a table.
* This routine includes dropping triggers, sequences,
@@ -1484,6 +1621,13 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
return;
vdbe_emit_fk_constraint_drop(parse_context, fk_name_dup, space_id);
}
+ /* Delete all CK constraints. */
+ struct ck_constraint *ck_constraint;
+ rlist_foreach_entry(ck_constraint, &space->ck_constraint, link) {
+ vdbe_emit_ck_constraint_drop(parse_context,
+ ck_constraint->def->name,
+ space_id);
+ }
/*
* Drop all _space and _index entries that refer to the
* table.
@@ -2770,8 +2914,7 @@ sqlSrcListDelete(sql * db, SrcList * pList)
*/
assert(pItem->space == NULL ||
!pItem->space->def->opts.is_temporary ||
- (pItem->space->index == NULL &&
- pItem->space->def->opts.checks == NULL));
+ pItem->space->index == NULL);
sql_select_delete(db, pItem->pSelect);
sql_expr_delete(db, pItem->pOn, false);
sqlIdListDelete(db, pItem->pUsing);
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 010695067..7b25d18b3 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -36,9 +36,10 @@
#include "sqlInt.h"
#include "tarantoolInt.h"
#include "vdbeInt.h"
-#include "box/schema.h"
+#include "box/ck_constraint.h"
#include "bit/bit.h"
#include "box/box.h"
+#include "box/schema.h"
enum field_type *
sql_index_type_str(struct sql *db, const struct index_def *idx_def)
@@ -932,34 +933,27 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space,
if (on_conflict == ON_CONFLICT_ACTION_DEFAULT)
on_conflict = ON_CONFLICT_ACTION_ABORT;
/* Test all CHECK constraints. */
- struct ExprList *checks = def->opts.checks;
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) {
+ if (!rlist_empty(&space->ck_constraint))
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 = sqlVdbeMakeLabel(v);
- sqlExprIfTrue(parse_context, expr, all_ok,
- SQL_JUMPIFNULL);
- if (on_conflict == ON_CONFLICT_ACTION_IGNORE) {
- sqlVdbeGoto(v, ignore_label);
- } else {
- char *name = checks->a[i].zName;
- if (name == NULL)
- name = def->name;
- sqlHaltConstraint(parse_context,
- SQL_CONSTRAINT_CHECK,
- on_conflict_check, name,
- P4_TRANSIENT,
- P5_ConstraintCheck);
- }
- sqlVdbeResolveLabel(v, all_ok);
+ 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);
+ } 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);
/*
@@ -1243,14 +1237,13 @@ xferOptimization(Parse * pParse, /* Parser context */
if (pSrcIdx == NULL)
return 0;
}
- /* Get server checks. */
- ExprList *pCheck_src = src->def->opts.checks;
- ExprList *pCheck_dest = dest->def->opts.checks;
- if (pCheck_dest != NULL &&
- sqlExprListCompare(pCheck_src, pCheck_dest, -1) != 0) {
- /* Tables have different CHECK constraints. Ticket #2252 */
+ /*
+ * Dissallow the transfer optimization if the are check
+ * constraints.
+ */
+ if (!rlist_empty(&dest->ck_constraint) ||
+ !rlist_empty(&src->ck_constraint))
return 0;
- }
/* Disallow the transfer optimization if the destination table constains
* any foreign key constraints. This is more restrictive than necessary.
* So the extra complication to make this rule less restrictive is probably
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 3a443a068..bc9f02599 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -290,7 +290,7 @@ ccons ::= check_constraint_def .
check_constraint_def ::= cconsname(N) CHECK LP expr(X) RP. {
create_ck_def_init(&pParse->create_ck_def, &N, &X);
- sql_add_check_constraint(pParse);
+ sql_create_check_contraint(pParse);
}
ccons ::= cconsname(N) REFERENCES nm(T) eidlist_opt(TA) matcharg(M) refargs(R). {
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index 5899a7e4e..6c1b6fddd 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -123,6 +123,22 @@ struct fk_constraint_parse {
struct rlist link;
};
+/**
+ * Structure representing check constraint appeared within
+ * CREATE TABLE statement. Used only during parsing.
+ * All allocations are performed on region, so no cleanups are
+ * required.
+ */
+struct ck_constraint_parse {
+ /**
+ * Check constraint declared in <CREATE TABLE ...>
+ * statement. Must be coded after space creation.
+ */
+ struct ck_constraint_def *ck_def;
+ /** Organize these structs into linked list. */
+ struct rlist link;
+};
+
/**
* Possible SQL index types. Note that PK and UNIQUE constraints
* are implemented as indexes and have their own types:
@@ -189,6 +205,13 @@ struct create_table_def {
* Foreign key constraint appeared in CREATE TABLE stmt.
*/
struct rlist new_fkey;
+ /**
+ * Number of CK constraints declared within
+ * CREATE TABLE statement.
+ */
+ uint32_t check_count;
+ /** Check constraint appeared in CREATE TABLE stmt. */
+ struct rlist new_check;
/** True, if table to be created has AUTOINCREMENT PK. */
bool has_autoinc;
};
@@ -437,6 +460,7 @@ create_table_def_init(struct create_table_def *table_def, struct Token *name,
create_entity_def_init(&table_def->base, ENTITY_TYPE_TABLE, NULL, name,
if_not_exists);
rlist_create(&table_def->new_fkey);
+ rlist_create(&table_def->new_check);
}
static inline void
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index d7376ae11..22aa62830 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -6440,7 +6440,12 @@ sql_expr_extract_select(struct Parse *parser, struct Select *select)
struct ExprList *expr_list = select->pEList;
assert(expr_list->nExpr == 1);
parser->parsed_ast_type = AST_TYPE_EXPR;
- parser->parsed_ast.expr = sqlExprDup(parser->db,
- expr_list->a->pExpr,
- EXPRDUP_REDUCE);
+ /*
+ * Extract a copy of parsed expression.
+ * We cannot use EXPRDUP_REDUCE flag in sqlExprDup call
+ * because some compiled Expr (like Checks expressions)
+ * may require further resolve with sqlResolveExprNames.
+ */
+ parser->parsed_ast.expr =
+ sqlExprDup(parser->db, expr_list->a->pExpr, 0);
}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 1b3f9afd1..0753705ec 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3346,7 +3346,7 @@ sqlAddPrimaryKey(struct Parse *parse);
* @param parser Parsing context.
*/
void
-sql_add_check_constraint(Parse *parser);
+sql_create_check_contraint(Parse *parser);
void sqlAddDefaultValue(Parse *, ExprSpan *);
void sqlAddCollateType(Parse *, Token *);
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 11ef7b97e..dbaebfefc 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -435,7 +435,6 @@ parser_space_delete(struct sql *db, struct space *space)
assert(space->def->opts.is_temporary);
for (uint32_t i = 0; i < space->index_count; ++i)
index_def_delete(space->index[i]->def);
- sql_expr_list_delete(db, space->def->opts.checks);
}
/**
diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
index c1e1490ca..7a72acda2 100755
--- a/test/app-tap/tarantoolctl.test.lua
+++ b/test/app-tap/tarantoolctl.test.lua
@@ -405,8 +405,8 @@ do
check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 1", "\n", 3)
check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 1 --replica 2", "\n", 3)
check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 2", "\n", 0)
- check_ctlcat_snap(test_i, dir, "--space=280", "---\n", 21)
- check_ctlcat_snap(test_i, dir, "--space=288", "---\n", 47)
+ check_ctlcat_snap(test_i, dir, "--space=280", "---\n", 22)
+ check_ctlcat_snap(test_i, dir, "--space=288", "---\n", 49)
end)
end)
diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result
index 379f6c51f..72cb9ea0f 100644
--- a/test/box-py/bootstrap.result
+++ b/test/box-py/bootstrap.result
@@ -4,7 +4,7 @@ box.internal.bootstrap()
box.space._schema:select{}
---
- - ['max_id', 511]
- - ['version', 2, 1, 3]
+ - ['version', 2, 2, 0]
...
box.space._cluster:select{}
---
@@ -78,6 +78,9 @@ box.space._space:select{}
{'name': 'is_deferred', 'type': 'boolean'}, {'name': 'match', 'type': 'string'},
{'name': 'on_delete', 'type': 'string'}, {'name': 'on_update', 'type': 'string'},
{'name': 'child_cols', 'type': 'array'}, {'name': 'parent_cols', 'type': 'array'}]]
+ - [357, 1, '_ck_constraint', 'memtx', 0, {}, [{'name': 'name', 'type': 'string'},
+ {'name': 'space_id', 'type': 'unsigned'}, {'name': 'is_deferred', 'type': 'boolean'},
+ {'name': 'code', 'type': 'str'}, {'name': 'language', 'type': 'str'}]]
...
box.space._index:select{}
---
@@ -130,6 +133,8 @@ box.space._index:select{}
- [340, 1, 'sequence', 'tree', {'unique': false}, [[1, 'unsigned']]]
- [356, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'unsigned']]]
- [356, 1, 'child_id', 'tree', {'unique': false}, [[1, 'unsigned']]]
+ - [357, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'unsigned']]]
+ - [357, 1, 'space_id', 'tree', {'unique': false}, [[1, 'unsigned']]]
...
box.space._user:select{}
---
diff --git a/test/box/access.result b/test/box/access.result
index 9c190240f..801112799 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -1487,6 +1487,9 @@ box.schema.user.grant('tester', 'read', 'space', '_trigger')
box.schema.user.grant('tester', 'read', 'space', '_fk_constraint')
---
...
+box.schema.user.grant('tester', 'read', 'space', '_ck_constraint')
+---
+...
box.session.su("tester")
---
...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 4baeb2ef6..2f62d6f53 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -554,6 +554,7 @@ box.schema.user.grant('tester', 'create' , 'sequence')
box.schema.user.grant('tester', 'read', 'space', '_sequence')
box.schema.user.grant('tester', 'read', 'space', '_trigger')
box.schema.user.grant('tester', 'read', 'space', '_fk_constraint')
+box.schema.user.grant('tester', 'read', 'space', '_ck_constraint')
box.session.su("tester")
-- successful create
s1 = box.schema.space.create("test_space")
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 36ebfae09..2e33e1431 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -818,6 +818,9 @@ box.space._space:select()
{'name': 'is_deferred', 'type': 'boolean'}, {'name': 'match', 'type': 'string'},
{'name': 'on_delete', 'type': 'string'}, {'name': 'on_update', 'type': 'string'},
{'name': 'child_cols', 'type': 'array'}, {'name': 'parent_cols', 'type': 'array'}]]
+ - [357, 1, '_ck_constraint', 'memtx', 0, {}, [{'name': 'name', 'type': 'string'},
+ {'name': 'space_id', 'type': 'unsigned'}, {'name': 'is_deferred', 'type': 'boolean'},
+ {'name': 'code', 'type': 'str'}, {'name': 'language', 'type': 'str'}]]
...
box.space._func:select()
---
diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result
index c6a2b22ed..c3d1e746b 100644
--- a/test/box/access_sysview.result
+++ b/test/box/access_sysview.result
@@ -230,11 +230,11 @@ box.session.su('guest')
...
#box.space._vspace:select{}
---
-- 22
+- 23
...
#box.space._vindex:select{}
---
-- 48
+- 50
...
#box.space._vuser:select{}
---
@@ -262,7 +262,7 @@ box.session.su('guest')
...
#box.space._vindex:select{}
---
-- 48
+- 50
...
#box.space._vuser:select{}
---
diff --git a/test/box/alter.result b/test/box/alter.result
index c1b1de135..44630557c 100644
--- a/test/box/alter.result
+++ b/test/box/alter.result
@@ -107,7 +107,7 @@ space = box.space[t[1]]
...
space.id
---
-- 357
+- 358
...
space.field_count
---
@@ -152,7 +152,7 @@ space_deleted
...
space:replace{0}
---
-- error: Space '357' does not exist
+- error: Space '358' does not exist
...
_index:insert{_space.id, 0, 'primary', 'tree', {unique=true}, {{0, 'unsigned'}}}
---
@@ -230,6 +230,8 @@ _index:select{}
- [340, 1, 'sequence', 'tree', {'unique': false}, [[1, 'unsigned']]]
- [356, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'unsigned']]]
- [356, 1, 'child_id', 'tree', {'unique': false}, [[1, 'unsigned']]]
+ - [357, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'unsigned']]]
+ - [357, 1, 'space_id', 'tree', {'unique': false}, [[1, 'unsigned']]]
...
-- modify indexes of a system space
_index:delete{_index.id, 0}
diff --git a/test/box/misc.result b/test/box/misc.result
index 4fcd13a78..5bf419d4f 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -523,6 +523,7 @@ t;
192: box.error.INDEX_DEF_UNSUPPORTED
193: box.error.CK_DEF_UNSUPPORTED
194: box.error.MULTIKEY_INDEX_MISMATCH
+ 195: box.error.CREATE_CK_CONSTRAINT
...
test_run:cmd("setopt delimiter ''");
---
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index b01afca7c..ede77c630 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, "Failed to execute SQL statement: CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
-- </check-1.3>
})
@@ -75,7 +75,7 @@ test:do_catchsql_test(
INSERT INTO t1 VALUES(4,3, 2);
]], {
-- <check-1.5>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_2_T1"
-- </check-1.5>
})
@@ -147,7 +147,7 @@ test:do_catchsql_test(
UPDATE t1 SET x=7 WHERE x==2
]], {
-- <check-1.12>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
-- </check-1.12>
})
@@ -167,7 +167,7 @@ test:do_catchsql_test(
UPDATE t1 SET x=5 WHERE x==2
]], {
-- <check-1.14>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
-- </check-1.14>
})
@@ -319,7 +319,7 @@ test:do_catchsql_test(
);
]], {
-- <check-3.1>
- 1, "Failed to create space 'T3': Subqueries are prohibited in a CHECK constraint definition"
+ 1, "Failed to create check constraint 'CK_CONSTRAINT_1_T3': Subqueries are prohibited in a ck constraint definition"
-- </check-3.1>
})
@@ -344,7 +344,7 @@ test:do_catchsql_test(
);
]], {
-- <check-3.3>
- 1, "Failed to create space 'T3': Can’t resolve field 'Q'"
+ 1, "Failed to create check constraint 'CK_CONSTRAINT_1_T3': Can’t resolve field 'Q'"
-- </check-3.3>
})
@@ -368,7 +368,7 @@ test:do_catchsql_test(
);
]], {
-- <check-3.5>
- 1, "Failed to create space 'T3': Field 'X' was not found in the space 'T2' format"
+ 1, "Failed to create check constraint 'CK_CONSTRAINT_1_T3': Field 'X' was not found in the space 'T2' format"
-- </check-3.5>
})
@@ -413,7 +413,7 @@ test:do_catchsql_test(
INSERT INTO t3 VALUES(111,222,333);
]], {
-- <check-3.9>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T3"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T3"
-- </check-3.9>
})
@@ -484,7 +484,7 @@ test:do_catchsql_test(
UPDATE t4 SET x=0, y=1;
]], {
-- <check-4.6>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T4"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T4"
-- </check-4.6>
})
@@ -504,7 +504,7 @@ test:do_catchsql_test(
UPDATE t4 SET x=0, y=2;
]], {
-- <check-4.9>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T4"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T4"
-- </check-4.9>
})
@@ -516,7 +516,7 @@ test:do_catchsql_test(
);
]], {
-- <check-5.1>
- 1, "Wrong space options (field 5): invalid expression specified (bindings are not allowed in DDL)"
+ 1, "Failed to create check constraint 'CK_CONSTRAINT_1_T5': bindings are not allowed in DDL"
-- </check-5.1>
})
@@ -528,7 +528,7 @@ test:do_catchsql_test(
);
]], {
-- <check-5.2>
- 1, "Wrong space options (field 5): invalid expression specified (bindings are not allowed in DDL)"
+ 1, "Failed to create check constraint 'CK_CONSTRAINT_1_T5': bindings are not allowed in DDL"
-- </check-5.2>
})
@@ -581,7 +581,7 @@ test:do_catchsql_test(
UPDATE OR FAIL t1 SET x=7-x, y=y+1;
]], {
-- <check-6.5>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
-- </check-6.5>
})
@@ -603,7 +603,7 @@ test:do_catchsql_test(
INSERT OR ROLLBACK INTO t1 VALUES(8,40.0, 10);
]], {
-- <check-6.7>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
-- </check-6.7>
})
@@ -636,7 +636,7 @@ test:do_catchsql_test(
REPLACE INTO t1 VALUES(6,7, 11);
]], {
-- <check-6.12>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T1"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
-- </check-6.12>
})
@@ -700,7 +700,7 @@ test:do_catchsql_test(
7.3,
" INSERT INTO t6 VALUES(11) ", {
-- <7.3>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T6"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T6"
-- </7.3>
})
diff --git a/test/sql-tap/fkey2.test.lua b/test/sql-tap/fkey2.test.lua
index 54e5059b3..695a379a6 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, "Failed to execute SQL statement: CHECK constraint failed: EF"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_EF"
-- </fkey2-3.2>
})
@@ -382,7 +382,7 @@ test:do_catchsql_test(
UPDATE ab SET a = 5;
]], {
-- <fkey2-3.4>
- 1, "Failed to execute SQL statement: CHECK constraint failed: EF"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_EF"
-- </fkey2-3.4>
})
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index 4e173b692..c2d17dc00 100755
--- a/test/sql-tap/sql-errors.test.lua
+++ b/test/sql-tap/sql-errors.test.lua
@@ -313,7 +313,7 @@ test:do_catchsql_test(
CREATE TABLE t27 (i INT PRIMARY KEY, CHECK(i < (SELECT * FROM t0)));
]], {
-- <sql-errors-1.27>
- 1,"Failed to create space 'T27': Subqueries are prohibited in a CHECK constraint definition"
+ 1,"Failed to create check constraint 'CK_CONSTRAINT_1_T27': Subqueries are prohibited in a ck constraint definition"
-- </sql-errors-1.27>
})
diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
index 5b793c0fc..066662f33 100755
--- a/test/sql-tap/table.test.lua
+++ b/test/sql-tap/table.test.lua
@@ -1221,7 +1221,7 @@ test:do_catchsql_test(
INSERT INTO T21 VALUES(1, -1, 1);
]], {
-- <table-21.3>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T21"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T21"
-- </table-21.3>
})
@@ -1231,7 +1231,7 @@ test:do_catchsql_test(
INSERT INTO T21 VALUES(1, 1, -1);
]], {
-- <table-21.4>
- 1, "Failed to execute SQL statement: CHECK constraint failed: T21"
+ 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_2_T21"
-- </table-21.4>
})
diff --git a/test/sql/checks.result b/test/sql/checks.result
index f7cddec43..f675c020b 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -18,8 +18,10 @@ box.execute('pragma sql_default_engine=\''..engine..'\'')
--
-- gh-3272: Move SQL CHECK into server
--
--- invalid expression
-opts = {checks = {{expr = 'X><5'}}}
+-- Until Tarantool version 2.2 check constraints were stored in
+-- space opts.
+-- Make sure that now this legacy option is ignored.
+opts = {checks = {{expr = 'X>5'}}}
---
...
format = {{name = 'X', type = 'unsigned'}}
@@ -30,89 +32,216 @@ t = {513, 1, 'test', 'memtx', 0, opts, format}
...
s = box.space._space:insert(t)
---
-- error: 'Wrong space options (field 5): invalid expression specified (Syntax error
- near ''<'')'
...
-opts = {checks = {{expr = 'X>5'}}}
+_ = box.space.test:create_index('pk')
---
...
-format = {{name = 'X', type = 'unsigned'}}
+-- Invalid expression test.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, false, 'X><5', 'SQL'})
---
+- error: 'Failed to create check constraint ''CK_CONSTRAINT_01'': Syntax error near
+ ''<'''
...
-t = {513, 1, 'test', 'memtx', 0, opts, format}
+-- Unexistent space test.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 550, false, 'X<5', 'SQL'})
---
+- error: Space '550' does not exist
...
-s = box.space._space:insert(t)
+-- Pass integer instead of expression.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, false, 666, 'SQL'})
+---
+- error: 'Tuple field 4 type does not match one required by operation: expected string'
+...
+-- Defered CK constraints are not supported.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, true, 'X<5', 'SQL'})
+---
+- error: Tarantool does not support deferred ck constraints
+...
+-- The only supperted language is SQL.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, false, 'X<5', 'LUA'})
+---
+- error: Unsupported language 'LUA' specified for function 'CK_CONSTRAINT_01'
+...
+-- Check constraints LUA creation test.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, false, 'X<5', 'SQL'})
---
+- ['CK_CONSTRAINT_01', 513, false, 'X<5', 'SQL']
...
-box.space._space:delete(513)
+box.space._ck_constraint:count({})
+---
+- 1
+...
+box.execute("INSERT INTO \"test\" VALUES(5);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_01'
+...
+box.space._ck_constraint:replace({'CK_CONSTRAINT_01', 513, false, 'X<=5', 'SQL'})
+---
+- ['CK_CONSTRAINT_01', 513, false, 'X<=5', 'SQL']
+...
+box.execute("INSERT INTO \"test\" VALUES(5);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO \"test\" VALUES(6);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_01'
+...
+-- Can't drop table with check constraints.
+box.space.test:delete({5})
+---
+- [5]
+...
+box.space.test.index.pk:drop()
+---
+...
+box.space._space:delete({513})
+---
+- error: 'Can''t drop space ''test'': the space has check constraints'
+...
+box.space._ck_constraint:delete({'CK_CONSTRAINT_01', 513})
+---
+- ['CK_CONSTRAINT_01', 513, false, 'X<=5', 'SQL']
+...
+box.space._space:delete({513})
---
- [513, 1, 'test', 'memtx', 0, {'checks': [{'expr': 'X>5'}]}, [{'name': 'X', 'type': 'unsigned'}]]
...
-opts = {checks = {{expr = 'X>5', name = 'ONE'}}}
+-- Create table with checks in sql.
+box.execute("CREATE TABLE t1(x INTEGER CONSTRAINT ONE CHECK( x<5 ), y REAL CONSTRAINT TWO CHECK( y>x ), z INTEGER PRIMARY KEY);")
---
+- row_count: 1
...
-format = {{name = 'X', type = 'unsigned'}}
+box.space._ck_constraint:count()
---
+- 2
...
-t = {513, 1, 'test', 'memtx', 0, opts, format}
+box.execute("INSERT INTO t1 VALUES (7, 1, 1)")
---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: ONE'
...
-s = box.space._space:insert(t)
+box.execute("INSERT INTO t1 VALUES (2, 1, 1)")
---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: TWO'
...
-box.space._space:delete(513)
+box.execute("INSERT INTO t1 VALUES (2, 4, 1)")
---
-- [513, 1, 'test', 'memtx', 0, {'checks': [{'name': 'ONE', 'expr': 'X>5'}]}, [{'name': 'X',
- 'type': 'unsigned'}]]
+- row_count: 1
...
--- extra invlalid field name
-opts = {checks = {{expr = 'X>5', name = 'ONE', extra = 'TWO'}}}
+box.execute("DROP TABLE t1")
---
+- row_count: 1
...
-format = {{name = 'X', type = 'unsigned'}}
+-- Test space creation rollback on spell error in ck constraint.
+box.execute("CREATE TABLE first (id FLOAT PRIMARY KEY CHECK(id < 5), a INT CONSTRAINT ONE CHECK(a >< 5));")
---
+- error: Syntax error near '<'
...
-t = {513, 1, 'test', 'memtx', 0, opts, format}
+box.space.FIRST == nil
---
+- true
...
-s = box.space._space:insert(t)
+box.space._ck_constraint:count() == 0
---
-- error: 'Wrong space options (field 5): invalid MsgPack map field ''extra'''
+- true
...
-opts = {checks = {{expr_invalid_label = 'X>5'}}}
+-- Ck constraints are disallowed for spaces having no format.
+s = box.schema.create_space('test', {engine = engine})
---
...
-format = {{name = 'X', type = 'unsigned'}}
+_ = s:create_index('pk')
---
...
-t = {513, 1, 'test', 'memtx', 0, opts, format}
+_ = box.space._ck_constraint:insert({'physics', s.id, false, 'X<Y', 'SQL'})
---
+- error: Tarantool does not support CK constraint for space without format
...
-s = box.space._space:insert(t)
+s:format({{name='X', type='integer'}, {name='Y', type='integer'}})
---
-- error: 'Wrong space options (field 5): invalid MsgPack map field ''expr_invalid_label'''
...
--- invalid field type
-opts = {checks = {{name = 123}}}
+_ = box.space._ck_constraint:insert({'physics', s.id, false, 'X<Y', 'SQL'})
---
...
-format = {{name = 'X', type = 'unsigned'}}
+box.execute("INSERT INTO \"test\" VALUES(2, 1);")
---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: physics'
...
-t = {513, 1, 'test', 'memtx', 0, opts, format}
+s:format({{name='Y', type='integer'}, {name='X', type='integer'}})
---
...
-s = box.space._space:insert(t)
+box.execute("INSERT INTO \"test\" VALUES(1, 2);")
---
-- error: 'Wrong space options (field 5): invalid MsgPack map field ''name'' type'
+- error: 'Failed to execute SQL statement: CHECK constraint failed: physics'
+...
+box.execute("INSERT INTO \"test\" VALUES(2, 1);")
+---
+- row_count: 1
+...
+s:truncate()
+---
+...
+box.execute("INSERT INTO \"test\" VALUES(1, 2);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: physics'
+...
+s:format({})
+---
+- error: Tarantool does not support CK constraint for space without format
+...
+s:format()
+---
+- [{'name': 'Y', 'type': 'integer'}, {'name': 'X', 'type': 'integer'}]
+...
+s:format({{name='Y1', type='integer'}, {name='X1', type='integer'}})
+---
+- error: 'Failed to create check constraint ''physics'': Can’t resolve field ''X'''
+...
+-- Ck constraint creation is forbidden for non-empty space
+s:insert({2, 1})
+---
+- [2, 1]
+...
+_ = box.space._ck_constraint:insert({'conflict', s.id, false, 'X>10', 'SQL'})
+---
+- error: 'Failed to create check constraint ''conflict'': referencing space must be
+ empty'
+...
+s:truncate()
+---
+...
+_ = box.space._ck_constraint:insert({'conflict', s.id, false, 'X>10', 'SQL'})
+---
+...
+box.execute("INSERT INTO \"test\" VALUES(1, 2);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: conflict'
+...
+box.execute("INSERT INTO \"test\" VALUES(11, 11);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: physics'
+...
+box.execute("INSERT INTO \"test\" VALUES(12, 11);")
+---
+- row_count: 1
+...
+s:drop()
+---
+...
+box.execute("CREATE TABLE T2(ID INT PRIMARY KEY, CONSTRAINT CK1 CHECK(ID > 0), CONSTRAINT CK1 CHECK(ID < 0))")
+---
+- error: Duplicate key exists in unique index 'primary' in space '_ck_constraint'
+...
+box.space._ck_constraint:select()
+---
+- []
...
--
-- gh-3611: Segfault on table creation with check referencing this table
--
box.execute("CREATE TABLE w2 (s1 INT PRIMARY KEY, CHECK ((SELECT COUNT(*) FROM w2) = 0));")
---
-- error: 'Failed to create space ''W2'': Space ''W2'' does not exist'
+- error: 'Failed to create check constraint ''CK_CONSTRAINT_1_W2'': Subqueries are
+ prohibited in a ck constraint definition'
...
box.execute("DROP TABLE w2;")
---
@@ -123,22 +252,8 @@ box.execute("DROP TABLE w2;")
--
box.execute("CREATE TABLE t5(x INT PRIMARY KEY, y INT, CHECK( x*y < ? ));")
---
-- error: 'Wrong space options (field 5): invalid expression specified (bindings are
- not allowed in DDL)'
-...
-opts = {checks = {{expr = '?>5', name = 'ONE'}}}
----
-...
-format = {{name = 'X', type = 'unsigned'}}
----
-...
-t = {513, 1, 'test', 'memtx', 0, opts, format}
----
-...
-s = box.space._space:insert(t)
----
-- error: 'Wrong space options (field 5): invalid expression specified (bindings are
- not allowed in DDL)'
+- error: 'Failed to create check constraint ''CK_CONSTRAINT_1_T5'': bindings are not
+ allowed in DDL'
...
test_run:cmd("clear filter")
---
diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua
index 5bfcf12f8..a6b0efcf0 100644
--- a/test/sql/checks.test.lua
+++ b/test/sql/checks.test.lua
@@ -8,41 +8,81 @@ box.execute('pragma sql_default_engine=\''..engine..'\'')
-- gh-3272: Move SQL CHECK into server
--
--- invalid expression
-opts = {checks = {{expr = 'X><5'}}}
-format = {{name = 'X', type = 'unsigned'}}
-t = {513, 1, 'test', 'memtx', 0, opts, format}
-s = box.space._space:insert(t)
-
+-- Until Tarantool version 2.2 check constraints were stored in
+-- space opts.
+-- Make sure that now this legacy option is ignored.
opts = {checks = {{expr = 'X>5'}}}
format = {{name = 'X', type = 'unsigned'}}
t = {513, 1, 'test', 'memtx', 0, opts, format}
s = box.space._space:insert(t)
-box.space._space:delete(513)
+_ = box.space.test:create_index('pk')
-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._space:delete(513)
+-- Invalid expression test.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, false, 'X><5', 'SQL'})
+-- Unexistent space test.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 550, false, 'X<5', 'SQL'})
+-- Pass integer instead of expression.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, false, 666, 'SQL'})
+-- Defered CK constraints are not supported.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, true, 'X<5', 'SQL'})
+-- The only supperted language is SQL.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, false, 'X<5', 'LUA'})
--- extra invlalid field name
-opts = {checks = {{expr = 'X>5', name = 'ONE', extra = 'TWO'}}}
-format = {{name = 'X', type = 'unsigned'}}
-t = {513, 1, 'test', 'memtx', 0, opts, format}
-s = box.space._space:insert(t)
+-- Check constraints LUA creation test.
+box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, false, 'X<5', 'SQL'})
+box.space._ck_constraint:count({})
-opts = {checks = {{expr_invalid_label = 'X>5'}}}
-format = {{name = 'X', type = 'unsigned'}}
-t = {513, 1, 'test', 'memtx', 0, opts, format}
-s = box.space._space:insert(t)
+box.execute("INSERT INTO \"test\" VALUES(5);")
+box.space._ck_constraint:replace({'CK_CONSTRAINT_01', 513, false, 'X<=5', 'SQL'})
+box.execute("INSERT INTO \"test\" VALUES(5);")
+box.execute("INSERT INTO \"test\" VALUES(6);")
+-- Can't drop table with check constraints.
+box.space.test:delete({5})
+box.space.test.index.pk:drop()
+box.space._space:delete({513})
+box.space._ck_constraint:delete({'CK_CONSTRAINT_01', 513})
+box.space._space:delete({513})
--- invalid field type
-opts = {checks = {{name = 123}}}
-format = {{name = 'X', type = 'unsigned'}}
-t = {513, 1, 'test', 'memtx', 0, opts, format}
-s = box.space._space:insert(t)
+-- Create table with checks in sql.
+box.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.execute("INSERT INTO t1 VALUES (7, 1, 1)")
+box.execute("INSERT INTO t1 VALUES (2, 1, 1)")
+box.execute("INSERT INTO t1 VALUES (2, 4, 1)")
+box.execute("DROP TABLE t1")
+
+-- Test space creation rollback on spell error in ck constraint.
+box.execute("CREATE TABLE first (id FLOAT PRIMARY KEY CHECK(id < 5), a INT CONSTRAINT ONE CHECK(a >< 5));")
+box.space.FIRST == nil
+box.space._ck_constraint:count() == 0
+
+-- Ck constraints are disallowed for spaces having no format.
+s = box.schema.create_space('test', {engine = engine})
+_ = s:create_index('pk')
+_ = box.space._ck_constraint:insert({'physics', s.id, false, 'X<Y', 'SQL'})
+s:format({{name='X', type='integer'}, {name='Y', type='integer'}})
+_ = box.space._ck_constraint:insert({'physics', s.id, false, 'X<Y', 'SQL'})
+box.execute("INSERT INTO \"test\" VALUES(2, 1);")
+s:format({{name='Y', type='integer'}, {name='X', type='integer'}})
+box.execute("INSERT INTO \"test\" VALUES(1, 2);")
+box.execute("INSERT INTO \"test\" VALUES(2, 1);")
+s:truncate()
+box.execute("INSERT INTO \"test\" VALUES(1, 2);")
+s:format({})
+s:format()
+s:format({{name='Y1', type='integer'}, {name='X1', type='integer'}})
+-- Ck constraint creation is forbidden for non-empty space
+s:insert({2, 1})
+_ = box.space._ck_constraint:insert({'conflict', s.id, false, 'X>10', 'SQL'})
+s:truncate()
+_ = box.space._ck_constraint:insert({'conflict', s.id, false, 'X>10', 'SQL'})
+box.execute("INSERT INTO \"test\" VALUES(1, 2);")
+box.execute("INSERT INTO \"test\" VALUES(11, 11);")
+box.execute("INSERT INTO \"test\" VALUES(12, 11);")
+s:drop()
+box.execute("CREATE TABLE T2(ID INT PRIMARY KEY, CONSTRAINT CK1 CHECK(ID > 0), CONSTRAINT CK1 CHECK(ID < 0))")
+box.space._ck_constraint:select()
--
-- gh-3611: Segfault on table creation with check referencing this table
@@ -55,10 +95,4 @@ box.execute("DROP TABLE w2;")
--
box.execute("CREATE TABLE t5(x INT PRIMARY KEY, y INT, CHECK( x*y < ? ));")
-opts = {checks = {{expr = '?>5', name = 'ONE'}}}
-format = {{name = 'X', type = 'unsigned'}}
-t = {513, 1, 'test', 'memtx', 0, opts, format}
-s = box.space._space:insert(t)
-
-
test_run:cmd("clear filter")
diff --git a/test/sql/errinj.result b/test/sql/errinj.result
index ee36a387b..e5bcbc9db 100644
--- a/test/sql/errinj.result
+++ b/test/sql/errinj.result
@@ -463,3 +463,137 @@ errinj.set("ERRINJ_SQL_NAME_NORMALIZATION", false)
---
- ok
...
+--
+-- Tests which are aimed at verifying work of commit/rollback
+-- triggers on _ck_constraint space.
+--
+s = box.schema.space.create('test', {format = {{name = 'X', type = 'unsigned'}}})
+---
+...
+pk = box.space.test:create_index('pk')
+---
+...
+errinj.set("ERRINJ_WAL_IO", true)
+---
+- ok
+...
+_ = box.space._ck_constraint:insert({'CK_CONSTRAINT_01', s.id, false, 'X<5', 'SQL'})
+---
+- error: Failed to write to disk
+...
+errinj.set("ERRINJ_WAL_IO", false)
+---
+- ok
+...
+_ = box.space._ck_constraint:insert({'CK_CONSTRAINT_01', s.id, false, 'X<5', 'SQL'})
+---
+...
+box.execute("INSERT INTO \"test\" VALUES(5);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_01'
+...
+errinj.set("ERRINJ_WAL_IO", true)
+---
+- ok
+...
+_ = box.space._ck_constraint:replace({'CK_CONSTRAINT_01', s.id, false, 'X<=5', 'SQL'})
+---
+- error: Failed to write to disk
+...
+errinj.set("ERRINJ_WAL_IO", false)
+---
+- ok
+...
+_ = box.space._ck_constraint:replace({'CK_CONSTRAINT_01', s.id, false, 'X<=5', 'SQL'})
+---
+...
+box.execute("INSERT INTO \"test\" VALUES(5);")
+---
+- row_count: 1
+...
+errinj.set("ERRINJ_WAL_IO", true)
+---
+- ok
+...
+_ = box.space._ck_constraint:delete({'CK_CONSTRAINT_01', s.id})
+---
+- error: Failed to write to disk
+...
+box.execute("INSERT INTO \"test\" VALUES(6);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_01'
+...
+errinj.set("ERRINJ_WAL_IO", false)
+---
+- ok
+...
+_ = box.space._ck_constraint:delete({'CK_CONSTRAINT_01', s.id})
+---
+...
+box.execute("INSERT INTO \"test\" VALUES(6);")
+---
+- row_count: 1
+...
+s:drop()
+---
+...
+--
+-- Test that failed space alter doesn't harm ck constraints
+--
+s = box.schema.create_space('test')
+---
+...
+_ = s:create_index('pk')
+---
+...
+s:format({{name='X', type='integer'}, {name='Y', type='integer'}})
+---
+...
+_ = box.space._ck_constraint:insert({'XlessY', s.id, false, 'X < Y', 'SQL'})
+---
+...
+_ = box.space._ck_constraint:insert({'Xgreater10', s.id, false, 'X > 10', 'SQL'})
+---
+...
+box.execute("INSERT INTO \"test\" VALUES(2, 1);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: Xgreater10'
+...
+box.execute("INSERT INTO \"test\" VALUES(20, 10);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: XlessY'
+...
+box.execute("INSERT INTO \"test\" VALUES(20, 100);")
+---
+- row_count: 1
+...
+s:truncate()
+---
+...
+errinj.set("ERRINJ_WAL_IO", true)
+---
+- ok
+...
+s:format({{name='Y', type='integer'}, {name='X', type='integer'}})
+---
+- error: Failed to write to disk
+...
+errinj.set("ERRINJ_WAL_IO", false)
+---
+- ok
+...
+box.execute("INSERT INTO \"test\" VALUES(2, 1);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: XlessY'
+...
+box.execute("INSERT INTO \"test\" VALUES(20, 10);")
+---
+- error: 'Failed to execute SQL statement: CHECK constraint failed: XlessY'
+...
+box.execute("INSERT INTO \"test\" VALUES(20, 100);")
+---
+- row_count: 1
+...
+s:drop()
+---
+...
diff --git a/test/sql/errinj.test.lua b/test/sql/errinj.test.lua
index 1aff6d77e..334f8bd28 100644
--- a/test/sql/errinj.test.lua
+++ b/test/sql/errinj.test.lua
@@ -149,3 +149,48 @@ box.execute("CREATE TABLE hello (id INT primary key,x INT,y INT);")
dummy_f = function(int) return 1 end
box.internal.sql_create_function("counter1", "INT", dummy_f, -1, false)
errinj.set("ERRINJ_SQL_NAME_NORMALIZATION", false)
+
+--
+-- Tests which are aimed at verifying work of commit/rollback
+-- triggers on _ck_constraint space.
+--
+s = box.schema.space.create('test', {format = {{name = 'X', type = 'unsigned'}}})
+pk = box.space.test:create_index('pk')
+
+errinj.set("ERRINJ_WAL_IO", true)
+_ = box.space._ck_constraint:insert({'CK_CONSTRAINT_01', s.id, false, 'X<5', 'SQL'})
+errinj.set("ERRINJ_WAL_IO", false)
+_ = box.space._ck_constraint:insert({'CK_CONSTRAINT_01', s.id, false, 'X<5', 'SQL'})
+box.execute("INSERT INTO \"test\" VALUES(5);")
+errinj.set("ERRINJ_WAL_IO", true)
+_ = box.space._ck_constraint:replace({'CK_CONSTRAINT_01', s.id, false, 'X<=5', 'SQL'})
+errinj.set("ERRINJ_WAL_IO", false)
+_ = box.space._ck_constraint:replace({'CK_CONSTRAINT_01', s.id, false, 'X<=5', 'SQL'})
+box.execute("INSERT INTO \"test\" VALUES(5);")
+errinj.set("ERRINJ_WAL_IO", true)
+_ = box.space._ck_constraint:delete({'CK_CONSTRAINT_01', s.id})
+box.execute("INSERT INTO \"test\" VALUES(6);")
+errinj.set("ERRINJ_WAL_IO", false)
+_ = box.space._ck_constraint:delete({'CK_CONSTRAINT_01', s.id})
+box.execute("INSERT INTO \"test\" VALUES(6);")
+s:drop()
+
+--
+-- Test that failed space alter doesn't harm ck constraints
+--
+s = box.schema.create_space('test')
+_ = s:create_index('pk')
+s:format({{name='X', type='integer'}, {name='Y', type='integer'}})
+_ = box.space._ck_constraint:insert({'XlessY', s.id, false, 'X < Y', 'SQL'})
+_ = box.space._ck_constraint:insert({'Xgreater10', s.id, false, 'X > 10', 'SQL'})
+box.execute("INSERT INTO \"test\" VALUES(2, 1);")
+box.execute("INSERT INTO \"test\" VALUES(20, 10);")
+box.execute("INSERT INTO \"test\" VALUES(20, 100);")
+s:truncate()
+errinj.set("ERRINJ_WAL_IO", true)
+s:format({{name='Y', type='integer'}, {name='X', type='integer'}})
+errinj.set("ERRINJ_WAL_IO", false)
+box.execute("INSERT INTO \"test\" VALUES(2, 1);")
+box.execute("INSERT INTO \"test\" VALUES(20, 10);")
+box.execute("INSERT INTO \"test\" VALUES(20, 100);")
+s:drop()
diff --git a/test/sql/gh-2981-check-autoinc.result b/test/sql/gh-2981-check-autoinc.result
index 9e347ca6b..7384c81e8 100644
--- a/test/sql/gh-2981-check-autoinc.result
+++ b/test/sql/gh-2981-check-autoinc.result
@@ -29,7 +29,7 @@ box.execute("insert into t1 values (18, null);")
...
box.execute("insert into t1(s2) values (null);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: T1'
+- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1'
...
box.execute("insert into t2 values (18, null);")
---
@@ -37,7 +37,7 @@ box.execute("insert into t2 values (18, null);")
...
box.execute("insert into t2(s2) values (null);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: T2'
+- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T2'
...
box.execute("insert into t2 values (24, null);")
---
@@ -45,7 +45,7 @@ box.execute("insert into t2 values (24, null);")
...
box.execute("insert into t2(s2) values (null);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: T2'
+- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T2'
...
box.execute("insert into t3 values (9, null)")
---
@@ -53,7 +53,7 @@ box.execute("insert into t3 values (9, null)")
...
box.execute("insert into t3(s2) values (null)")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: T3'
+- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T3'
...
box.execute("DROP TABLE t1")
---
diff --git a/test/sql/types.result b/test/sql/types.result
index bc4518c01..582785413 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -709,7 +709,7 @@ box.execute("CREATE TABLE t1 (id INT PRIMARY KEY, a BOOLEAN CHECK (a = true));")
...
box.execute("INSERT INTO t1 VALUES (1, false);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: T1'
+- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1'
...
box.execute("INSERT INTO t1 VALUES (1, true);")
---
diff --git a/test/sql/upgrade.result b/test/sql/upgrade.result
index 3a55f7c53..f0997e17f 100644
--- a/test/sql/upgrade.result
+++ b/test/sql/upgrade.result
@@ -188,6 +188,25 @@ i[1].opts.sql == nil
---
- true
...
+box.space._space:get(s.id).flags.checks == nil
+---
+- true
+...
+check = box.space._ck_constraint:select()[1]
+---
+...
+check ~= nil
+---
+- true
+...
+check.name
+---
+- CK_CONSTRAINT_1_T5
+...
+check.code
+---
+- x < 2
+...
s:drop()
---
...
diff --git a/test/sql/upgrade.test.lua b/test/sql/upgrade.test.lua
index b76a8f373..37425ae21 100644
--- a/test/sql/upgrade.test.lua
+++ b/test/sql/upgrade.test.lua
@@ -62,6 +62,11 @@ s ~= nil
i = box.space._index:select(s.id)
i ~= nil
i[1].opts.sql == nil
+box.space._space:get(s.id).flags.checks == nil
+check = box.space._ck_constraint:select()[1]
+check ~= nil
+check.name
+check.code
s:drop()
test_run:switch('default')
diff --git a/test/wal_off/alter.result b/test/wal_off/alter.result
index ee280fcbb..8040efa1a 100644
--- a/test/wal_off/alter.result
+++ b/test/wal_off/alter.result
@@ -28,7 +28,7 @@ end;
...
#spaces;
---
-- 65505
+- 65504
...
-- cleanup
for k, v in pairs(spaces) do
--
2.21.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] [PATCH v4 2/4] box: run check constraint tests on space alter
2019-05-16 13:56 [tarantool-patches] [PATCH v4 0/4] box: run checks on insertions in LUA spaces Kirill Shcherbatov
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 1/4] schema: add new system space for CHECK constraints Kirill Shcherbatov
@ 2019-05-16 13:56 ` Kirill Shcherbatov
2019-05-19 16:02 ` [tarantool-patches] " Vladislav Shpilevoy
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 3/4] box: introduce column_mask for ck constraint Kirill Shcherbatov
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 4/4] box: user-friendly interface to manage ck constraints Kirill Shcherbatov
3 siblings, 1 reply; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-16 13:56 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
To perform ck constraints tests before insert or update space operation,
we use precompiled VDBE machine is associated with each ck constraint,
that is executed in on_replace trigger. Each ck constraint VDBE code is
consists of
1) prologue code that maps new(or updated) tuple fields via bindings,
2) ck constraint code is generated by CK constraint AST.
In case of ck constraint error the transaction is aborted and ck
constraint error is handled as diag message.
Each ck constraint use own on_replace trigger.
Needed for #3691
---
src/box/alter.cc | 58 ++++++-
src/box/ck_constraint.c | 158 ++++++++++++++++++-
src/box/ck_constraint.h | 16 +-
src/box/errcode.h | 1 +
src/box/sql/insert.c | 92 +++--------
src/box/sql/sqlInt.h | 26 +++
src/box/sql/vdbeapi.c | 8 -
test/box/misc.result | 1 +
test/sql-tap/check.test.lua | 32 ++--
test/sql-tap/fkey2.test.lua | 4 +-
test/sql-tap/table.test.lua | 12 +-
test/sql/checks.result | 217 ++++++++++++++++++++++++--
test/sql/checks.test.lua | 61 ++++++++
test/sql/errinj.result | 18 ++-
test/sql/gh-2981-check-autoinc.result | 12 +-
test/sql/types.result | 3 +-
16 files changed, 581 insertions(+), 138 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index e65c49d14..746ce62f6 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1410,14 +1410,29 @@ void
RebuildCkConstraints::alter(struct alter_space *alter)
{
rlist_swap(&alter->new_space->ck_constraint, &ck_constraint);
+ struct ck_constraint *ck;
+ rlist_foreach_entry(ck, &alter->old_space->ck_constraint, link)
+ trigger_clear(&ck->trigger);
rlist_swap(&ck_constraint, &alter->old_space->ck_constraint);
+ rlist_foreach_entry(ck, &alter->new_space->ck_constraint, link) {
+ /**
+ * Triggers would be swapped later on
+ * alter_space_do.
+ */
+ trigger_add(&alter->old_space->on_replace, &ck->trigger);
+ }
}
void
RebuildCkConstraints::rollback(struct alter_space *alter)
{
rlist_swap(&alter->old_space->ck_constraint, &ck_constraint);
+ struct ck_constraint *ck;
+ rlist_foreach_entry(ck, &alter->new_space->ck_constraint, link)
+ trigger_clear(&ck->trigger);
rlist_swap(&ck_constraint, &alter->new_space->ck_constraint);
+ rlist_foreach_entry(ck, &alter->old_space->ck_constraint, link)
+ trigger_add(&alter->new_space->on_replace, &ck->trigger);
}
RebuildCkConstraints::~RebuildCkConstraints()
@@ -1435,6 +1450,35 @@ RebuildCkConstraints::~RebuildCkConstraints()
}
}
+/**
+ * Move CK constraints from old space to the new one.
+ * Despite RebuildCkConstraints, this operation doesn't perform
+ * objects rebuild. This may be used in scenarios where space
+ * format doesn't change i.e. in alter index or space trim
+ * requests.
+ */
+class MoveCkConstraints: public AlterSpaceOp
+{
+public:
+ MoveCkConstraints(struct alter_space *alter) : AlterSpaceOp(alter) {}
+ virtual void alter(struct alter_space *alter);
+ virtual void rollback(struct alter_space *alter);
+};
+
+void
+MoveCkConstraints::alter(struct alter_space *alter)
+{
+ rlist_swap(&alter->new_space->ck_constraint,
+ &alter->old_space->ck_constraint);
+}
+
+void
+MoveCkConstraints::rollback(struct alter_space *alter)
+{
+ rlist_swap(&alter->new_space->ck_constraint,
+ &alter->old_space->ck_constraint);
+}
+
/* }}} */
/**
@@ -2145,8 +2189,8 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
* old space.
*/
alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1);
+ (void) new MoveCkConstraints(alter);
/* Add an op to update schema_version on commit. */
- (void) new RebuildCkConstraints(alter);
(void) new UpdateSchemaVersion(alter);
alter_space_do(txn, alter);
scoped_guard.is_active = false;
@@ -2214,7 +2258,7 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
(void) new TruncateIndex(alter, old_index->def->iid);
}
- (void) new RebuildCkConstraints(alter);
+ (void) new MoveCkConstraints(alter);
alter_space_do(txn, alter);
scoped_guard.is_active = false;
}
@@ -4160,12 +4204,14 @@ on_replace_ck_constraint_rollback(struct trigger *trigger, void *event)
assert(space_ck_constraint_by_name(space,
ck->def->name, strlen(ck->def->name)) == NULL);
rlist_add_entry(&space->ck_constraint, ck, link);
+ trigger_add(&space->on_replace, &ck->trigger);
} else if (stmt->new_tuple != NULL && stmt->old_tuple == NULL) {
/* Rollback INSERT check constraint. */
assert(space != NULL);
assert(space_ck_constraint_by_name(space,
ck->def->name, strlen(ck->def->name)) != NULL);
rlist_del_entry(ck, link);
+ trigger_clear(&ck->trigger);
ck_constraint_delete(ck);
} else {
/* Rollback REPLACE check constraint. */
@@ -4175,7 +4221,9 @@ on_replace_ck_constraint_rollback(struct trigger *trigger, void *event)
space_ck_constraint_by_name(space, name, strlen(name));
assert(new_ck != NULL);
rlist_del_entry(new_ck, link);
+ trigger_clear(&new_ck->trigger);
rlist_add_entry(&space->ck_constraint, ck, link);
+ trigger_add(&space->on_replace, &ck->trigger);
ck_constraint_delete(new_ck);
}
}
@@ -4242,9 +4290,12 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event)
const char *name = new_ck_constraint->def->name;
struct ck_constraint *old_ck_constraint =
space_ck_constraint_by_name(space, name, strlen(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->on_replace, &new_ck_constraint->trigger);
on_commit->data = old_tuple == NULL ? new_ck_constraint :
old_ck_constraint;
on_rollback->data = on_commit->data;
@@ -4260,6 +4311,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 db012837b..43798be76 100644
--- a/src/box/ck_constraint.c
+++ b/src/box/ck_constraint.c
@@ -29,10 +29,15 @@
* SUCH DAMAGE.
*/
#include "box/session.h"
+#include "bind.h"
#include "ck_constraint.h"
#include "errcode.h"
+#include "schema.h"
+#include "session.h"
#include "sql.h"
#include "sql/sqlInt.h"
+#include "sql/vdbeInt.h"
+#include "tuple.h"
const char *ck_constraint_language_strs[] = {"SQL"};
@@ -57,6 +62,138 @@ ck_constraint_resolve_field_names(struct Expr *expr,
return rc;
}
+/**
+ * Create VDBE machine for ck constraint by given definition and
+ * expression AST. The generated instructions consist of
+ * prologue code that maps tuple fields via bindings and ck
+ * constraint code which implements given expression.
+ * In case of ck constraint error during VDBE execution, it is
+ * aborted and error is handled as diag message.
+ * @param ck_constraint_def Check constraint definition to prepare
+ * error description.
+ * @param expr Check constraint expression AST is built for
+ * given @ck_constraint_def, see for
+ * (sql_expr_compile +
+ * ck_constraint_resolve_space_def) implementation.
+ * @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.
+ */
+static struct sql_stmt *
+ck_constraint_program_compile(struct ck_constraint_def *ck_constraint_def,
+ struct Expr *expr, struct space_def *space_def)
+{
+ struct sql *db = sql_get();
+ struct Parse parser;
+ sql_parser_create(&parser, db, default_flags);
+ struct Vdbe *v = sqlGetVdbe(&parser);
+ if (v == NULL) {
+ diag_set(OutOfMemory, sizeof(struct Vdbe), "sqlGetVdbe",
+ "vdbe");
+ return NULL;
+ }
+ /*
+ * Generate a prologue code that introduces variables to
+ * bind tuple fields there before execution.
+ */
+ uint32_t field_count = space_def->field_count;
+ int bind_tuple_reg = sqlGetTempRange(&parser, field_count);
+ for (uint32_t i = 0; i < field_count; i++) {
+ sqlVdbeAddOp2(v, OP_Variable, ++parser.nVar,
+ bind_tuple_reg + i);
+ }
+ /* Generate ck constraint test code. */
+ vdbe_emit_ck_constraint(&parser, expr, ck_constraint_def->name,
+ ck_constraint_def->expr_str, bind_tuple_reg);
+
+ /* Clean-up and restore user-defined sql context. */
+ bool is_error = parser.is_aborted;
+ sql_finish_coding(&parser);
+ sql_parser_destroy(&parser);
+
+ if (is_error) {
+ diag_set(ClientError, ER_CREATE_CK_CONSTRAINT,
+ ck_constraint_def->name,
+ box_error_message(box_error_last()));
+ sql_finalize((struct sql_stmt *) v);
+ return NULL;
+ }
+ return (struct sql_stmt *) v;
+}
+
+/**
+ * Run bytecode implementing check constraint on new tuple
+ * before insert or replace in space space_def.
+ * @param ck_constraint Check constraint to run.
+ * @param space_def The space definition of the space this check
+ * constraint is constructed for.
+ * @param new_tuple The tuple to be inserted in space.
+ * @retval 0 if check constraint test is passed, -1 otherwise.
+ */
+static int
+ck_constraint_program_run(struct ck_constraint *ck_constraint,
+ const char *new_tuple)
+{
+ /*
+ * Prepare parameters for checks->stmt execution:
+ * Map new tuple fields to Vdbe memory variables in range:
+ * [1, field_count]
+ */
+ struct space *space = space_by_id(ck_constraint->space_id);
+ assert(space != NULL);
+ /*
+ * When last format fields are nullable, they are
+ * 'optional' i.e. they may not be present in the tuple.
+ */
+ uint32_t tuple_field_count = mp_decode_array(&new_tuple);
+ uint32_t field_count =
+ MIN(tuple_field_count, space->def->field_count);
+ for (uint32_t i = 0; i < field_count; i++) {
+ struct sql_bind bind;
+ if (sql_bind_decode(&bind, i + 1, &new_tuple) != 0 ||
+ sql_bind_column(ck_constraint->stmt, &bind, i + 1) != 0) {
+ diag_set(ClientError, ER_CK_CONSTRAINT_FAILED,
+ ck_constraint->def->name,
+ ck_constraint->def->expr_str);
+ return -1;
+ }
+ }
+ /* Checks VDBE can't expire, reset expired flag and go. */
+ struct Vdbe *v = (struct Vdbe *) ck_constraint->stmt;
+ v->expired = 0;
+ while (sql_step(ck_constraint->stmt) == SQL_ROW) {}
+ /*
+ * Get VDBE execution state and reset VM to run it
+ * next time.
+ */
+ return sql_reset(ck_constraint->stmt) != SQL_OK ? -1 : 0;
+}
+
+/**
+ * Ck constraint trigger function. It ss expected to be executed
+ * in space::on_replace trigger.
+ *
+ * It extracts all ck constraint required context from event and
+ * run bytecode implementing check constraint to test a new tuple
+ * before it will be inserted in destination space.
+ */
+static void
+ck_constraint_on_replace_trigger(struct trigger *trigger, void *event)
+{
+ struct ck_constraint *ck_constraint =
+ (struct ck_constraint *) trigger->data;
+ struct txn *txn = (struct txn *) event;
+ struct txn_stmt *stmt = txn_current_stmt(txn);
+ assert(stmt != NULL);
+ struct tuple *new_tuple = stmt->new_tuple;
+ if (new_tuple == NULL)
+ return;
+ if (ck_constraint_program_run(ck_constraint,
+ tuple_data(new_tuple)) != 0)
+ diag_raise();
+}
+
struct ck_constraint *
ck_constraint_new(struct ck_constraint_def *ck_constraint_def,
struct space_def *space_def)
@@ -73,23 +210,33 @@ ck_constraint_new(struct ck_constraint_def *ck_constraint_def,
return NULL;
}
ck_constraint->def = NULL;
+ ck_constraint->stmt = NULL;
ck_constraint->space_id = space_def->id;
rlist_create(&ck_constraint->link);
- ck_constraint->expr =
+ trigger_create(&ck_constraint->trigger,
+ ck_constraint_on_replace_trigger, ck_constraint,
+ NULL);
+ struct Expr *expr =
sql_expr_compile(sql_get(), ck_constraint_def->expr_str,
strlen(ck_constraint_def->expr_str));
- if (ck_constraint->expr == NULL ||
- ck_constraint_resolve_field_names(ck_constraint->expr,
- space_def) != 0) {
+ if (expr == NULL ||
+ ck_constraint_resolve_field_names(expr, space_def) != 0) {
diag_set(ClientError, ER_CREATE_CK_CONSTRAINT,
ck_constraint_def->name,
box_error_message(box_error_last()));
goto error;
}
+ ck_constraint->stmt =
+ ck_constraint_program_compile(ck_constraint_def, expr,
+ space_def);
+ if (ck_constraint->stmt == NULL)
+ goto error;
+ sql_expr_delete(sql_get(), expr, false);
ck_constraint->def = ck_constraint_def;
return ck_constraint;
error:
+ sql_expr_delete(sql_get(), expr, false);
ck_constraint_delete(ck_constraint);
return NULL;
}
@@ -97,7 +244,8 @@ error:
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);
free(ck_constraint->def);
TRASH(ck_constraint);
free(ck_constraint);
diff --git a/src/box/ck_constraint.h b/src/box/ck_constraint.h
index 9c72d5977..a948a074e 100644
--- a/src/box/ck_constraint.h
+++ b/src/box/ck_constraint.h
@@ -32,6 +32,7 @@
*/
#include <stdint.h>
+#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;
/** The supported language of the ck constraint. */
@@ -81,12 +83,16 @@ struct ck_constraint {
/** The check constraint definition. */
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 sql_resolve_self_reference for
- * space with space[ck_constraint::space_id] definition.
+ * Precompiled reusable VDBE program for processing check
+ * constraints and setting bad exitcode and error
+ * message when ck condition unsatisfied.
*/
- struct Expr *expr;
+ struct sql_stmt *stmt;
+ /**
+ * Trigger object executing check constraint before
+ * insert and replace operations.
+ */
+ struct trigger trigger;
/**
* The id of the space this check constraint is
* built for.
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 1f7c81693..e2ec240d2 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -248,6 +248,7 @@ struct errcode_record {
/*193 */_(ER_CK_DEF_UNSUPPORTED, "%s are prohibited in a ck constraint definition") \
/*194 */_(ER_MULTIKEY_INDEX_MISMATCH, "Field %s is used as multikey in one index and as single key in another") \
/*195 */_(ER_CREATE_CK_CONSTRAINT, "Failed to create check constraint '%s': %s") \
+ /*196 */_(ER_CK_CONSTRAINT_FAILED, "Check constraint failed '%s': %s") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 7b25d18b3..8409ab343 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -812,51 +812,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, const char *expr_str,
+ 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",
+ ck_constraint_name));
+ int check_is_passed = sqlVdbeMakeLabel(v);
+ sqlExprIfTrue(parser, expr, check_is_passed, SQL_JUMPIFNULL);
+ sqlMayAbort(parser);
+ const char *fmt = tnt_errcode_desc(ER_CK_CONSTRAINT_FAILED);
+ const char *error_msg = tt_sprintf(fmt, ck_constraint_name, expr_str);
+ 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", ck_constraint_name));
+ sqlVdbeResolveLabel(v, check_is_passed);
}
void
@@ -926,35 +901,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);
- } 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 0753705ec..d353d7906 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -605,6 +605,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 */
@@ -3894,6 +3905,21 @@ 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, const char *expr_str,
+ 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 d2868567b..5b3ac4b6a 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/box/misc.result b/test/box/misc.result
index 5bf419d4f..33e41b55e 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -524,6 +524,7 @@ t;
193: box.error.CK_DEF_UNSUPPORTED
194: box.error.MULTIKEY_INDEX_MISMATCH
195: box.error.CREATE_CK_CONSTRAINT
+ 196: box.error.CK_CONSTRAINT_FAILED
...
test_run:cmd("setopt delimiter ''");
---
diff --git a/test/sql-tap/check.test.lua b/test/sql-tap/check.test.lua
index ede77c630..e1334f435 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, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
+ 1, "Failed to execute SQL statement: Check constraint failed 'CK_CONSTRAINT_1_T1': x<5"
-- </check-1.3>
})
@@ -75,7 +75,7 @@ test:do_catchsql_test(
INSERT INTO t1 VALUES(4,3, 2);
]], {
-- <check-1.5>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_2_T1"
+ 1, "Failed to execute SQL statement: Check constraint failed 'CK_CONSTRAINT_2_T1': y>x"
-- </check-1.5>
})
@@ -147,7 +147,7 @@ test:do_catchsql_test(
UPDATE t1 SET x=7 WHERE x==2
]], {
-- <check-1.12>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
+ 1, "Check constraint failed 'CK_CONSTRAINT_1_T1': x<5"
-- </check-1.12>
})
@@ -167,7 +167,7 @@ test:do_catchsql_test(
UPDATE t1 SET x=5 WHERE x==2
]], {
-- <check-1.14>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
+ 1, "Check constraint failed 'CK_CONSTRAINT_1_T1': x<5"
-- </check-1.14>
})
@@ -206,9 +206,9 @@ test:do_execsql_test(
[[
CREATE TABLE t2(
id INT primary key,
- x INTEGER CONSTRAINT one CHECK( typeof(coalesce(x,0))=='integer'),
+ x SCALAR CONSTRAINT one CHECK( typeof(coalesce(x,0))=='integer'),
y REAL CONSTRAINT two CHECK( typeof(coalesce(y,0.1))=='number' ),
- z TEXT CONSTRAINT three CHECK( typeof(coalesce(z,''))=='string' )
+ z SCALAR CONSTRAINT three CHECK( typeof(coalesce(z,''))=='string' )
);
]], {
-- <check-2.1>
@@ -246,7 +246,7 @@ test:do_catchsql_test(
INSERT INTO t2 VALUES(3, 1.1, NULL, NULL);
]], {
-- <check-2.4>
- 1, "Failed to execute SQL statement: CHECK constraint failed: ONE"
+ 1, "Failed to execute SQL statement: Check constraint failed 'ONE': typeof(coalesce(x,0))=='integer'"
-- </check-2.4>
})
@@ -256,7 +256,7 @@ test:do_catchsql_test(
INSERT INTO t2 VALUES(4, NULL, 5, NULL);
]], {
-- <check-2.5>
- 1, "Failed to execute SQL statement: CHECK constraint failed: TWO"
+ 1, "Failed to execute SQL statement: Check constraint failed 'TWO': typeof(coalesce(y,0.1))=='number'"
-- </check-2.5>
})
@@ -266,7 +266,7 @@ test:do_catchsql_test(
INSERT INTO t2 VALUES(5, NULL, NULL, 3.14159);
]], {
-- <check-2.6>
- 1, "Failed to execute SQL statement: CHECK constraint failed: THREE"
+ 1, "Failed to execute SQL statement: Check constraint failed 'THREE': typeof(coalesce(z,''))=='string'"
-- </check-2.6>
})
@@ -413,7 +413,7 @@ test:do_catchsql_test(
INSERT INTO t3 VALUES(111,222,333);
]], {
-- <check-3.9>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T3"
+ 1, "Failed to execute SQL statement: Check constraint failed 'CK_CONSTRAINT_1_T3': t3.x<25"
-- </check-3.9>
})
@@ -484,7 +484,7 @@ test:do_catchsql_test(
UPDATE t4 SET x=0, y=1;
]], {
-- <check-4.6>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T4"
+ 1, "Check constraint failed 'CK_CONSTRAINT_1_T4': x+y==11 OR x*y==12 OR x/y BETWEEN 5 AND 8 OR -x==y+10"
-- </check-4.6>
})
@@ -504,7 +504,7 @@ test:do_catchsql_test(
UPDATE t4 SET x=0, y=2;
]], {
-- <check-4.9>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T4"
+ 1, "Check constraint failed 'CK_CONSTRAINT_1_T4': x+y==11 OR x*y==12 OR x/y BETWEEN 5 AND 8 OR -x==y+10"
-- </check-4.9>
})
@@ -581,7 +581,7 @@ test:do_catchsql_test(
UPDATE OR FAIL t1 SET x=7-x, y=y+1;
]], {
-- <check-6.5>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
+ 1, "Check constraint failed 'CK_CONSTRAINT_1_T1': x<5"
-- </check-6.5>
})
@@ -603,7 +603,7 @@ test:do_catchsql_test(
INSERT OR ROLLBACK INTO t1 VALUES(8,40.0, 10);
]], {
-- <check-6.7>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
+ 1, "Failed to execute SQL statement: Check constraint failed 'CK_CONSTRAINT_1_T1': x<5"
-- </check-6.7>
})
@@ -636,7 +636,7 @@ test:do_catchsql_test(
REPLACE INTO t1 VALUES(6,7, 11);
]], {
-- <check-6.12>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1"
+ 1, "Failed to execute SQL statement: Check constraint failed 'CK_CONSTRAINT_1_T1': x<5"
-- </check-6.12>
})
@@ -700,7 +700,7 @@ test:do_catchsql_test(
7.3,
" INSERT INTO t6 VALUES(11) ", {
-- <7.3>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T6"
+ 1, "Failed to execute SQL statement: Check constraint failed 'CK_CONSTRAINT_1_T6': myfunc(a)"
-- </7.3>
})
diff --git a/test/sql-tap/fkey2.test.lua b/test/sql-tap/fkey2.test.lua
index 695a379a6..def5f8321 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, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_EF"
+ 1, "Check constraint failed 'CK_CONSTRAINT_1_EF': e!=5"
-- </fkey2-3.2>
})
@@ -382,7 +382,7 @@ test:do_catchsql_test(
UPDATE ab SET a = 5;
]], {
-- <fkey2-3.4>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_EF"
+ 1, "Check constraint failed 'CK_CONSTRAINT_1_EF': e!=5"
-- </fkey2-3.4>
})
diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua
index 066662f33..19df1d5df 100755
--- a/test/sql-tap/table.test.lua
+++ b/test/sql-tap/table.test.lua
@@ -1218,20 +1218,20 @@ test:do_catchsql_test(
test:do_catchsql_test(
"table-21.3",
[[
- INSERT INTO T21 VALUES(1, -1, 1);
+ INSERT INTO T21 VALUES(2, -1, 1);
]], {
-- <table-21.3>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T21"
+ 1, "Failed to execute SQL statement: Check constraint failed 'CK_CONSTRAINT_1_T21': B > 0"
-- </table-21.3>
})
test:do_catchsql_test(
"table-21.4",
[[
- INSERT INTO T21 VALUES(1, 1, -1);
+ INSERT INTO T21 VALUES(2, 1, -1);
]], {
-- <table-21.4>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_2_T21"
+ 1, "Failed to execute SQL statement: Check constraint failed 'CK_CONSTRAINT_2_T21': C > 0"
-- </table-21.4>
})
@@ -1372,7 +1372,7 @@ test:do_catchsql_test(
INSERT INTO T28 VALUES(0);
]], {
-- <table-22.10>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CHECK1"
+ 1, "Failed to execute SQL statement: Check constraint failed 'CHECK1': id != 0"
-- </table-22.10>
})
@@ -1382,7 +1382,7 @@ test:do_catchsql_test(
INSERT INTO T28 VALUES(9);
]], {
-- <table-22.11>
- 1, "Failed to execute SQL statement: CHECK constraint failed: CHECK2"
+ 1, "Failed to execute SQL statement: Check constraint failed 'CHECK2': id > 10"
-- </table-22.11>
})
diff --git a/test/sql/checks.result b/test/sql/checks.result
index f675c020b..b74572e3e 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -73,7 +73,12 @@ box.space._ck_constraint:count({})
...
box.execute("INSERT INTO \"test\" VALUES(5);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_01'
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_01'':
+ X<5'
+...
+box.space.test:insert({5})
+---
+- error: 'Check constraint failed ''CK_CONSTRAINT_01'': X<5'
...
box.space._ck_constraint:replace({'CK_CONSTRAINT_01', 513, false, 'X<=5', 'SQL'})
---
@@ -85,7 +90,12 @@ box.execute("INSERT INTO \"test\" VALUES(5);")
...
box.execute("INSERT INTO \"test\" VALUES(6);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_01'
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_01'':
+ X<=5'
+...
+box.space.test:insert({6})
+---
+- error: 'Check constraint failed ''CK_CONSTRAINT_01'': X<=5'
...
-- Can't drop table with check constraints.
box.space.test:delete({5})
@@ -118,16 +128,28 @@ box.space._ck_constraint:count()
...
box.execute("INSERT INTO t1 VALUES (7, 1, 1)")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: ONE'
+- error: 'Failed to execute SQL statement: Check constraint failed ''ONE'': x<5'
+...
+box.space.T1:insert({7, 1, 1})
+---
+- error: 'Check constraint failed ''ONE'': x<5'
...
box.execute("INSERT INTO t1 VALUES (2, 1, 1)")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: TWO'
+- error: 'Failed to execute SQL statement: Check constraint failed ''TWO'': y>x'
+...
+box.space.T1:insert({2, 1, 1})
+---
+- error: 'Check constraint failed ''TWO'': y>x'
...
box.execute("INSERT INTO t1 VALUES (2, 4, 1)")
---
- row_count: 1
...
+box.space.T1:update({1}, {{'+', 1, 5}})
+---
+- error: 'Check constraint failed ''ONE'': x<5'
+...
box.execute("DROP TABLE t1")
---
- row_count: 1
@@ -164,14 +186,14 @@ _ = box.space._ck_constraint:insert({'physics', s.id, false, 'X<Y', 'SQL'})
...
box.execute("INSERT INTO \"test\" VALUES(2, 1);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: physics'
+- error: 'Failed to execute SQL statement: Check constraint failed ''physics'': X<Y'
...
s:format({{name='Y', type='integer'}, {name='X', type='integer'}})
---
...
box.execute("INSERT INTO \"test\" VALUES(1, 2);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: physics'
+- error: 'Failed to execute SQL statement: Check constraint failed ''physics'': X<Y'
...
box.execute("INSERT INTO \"test\" VALUES(2, 1);")
---
@@ -182,7 +204,7 @@ s:truncate()
...
box.execute("INSERT INTO \"test\" VALUES(1, 2);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: physics'
+- error: 'Failed to execute SQL statement: Check constraint failed ''physics'': X<Y'
...
s:format({})
---
@@ -214,11 +236,11 @@ _ = box.space._ck_constraint:insert({'conflict', s.id, false, 'X>10', 'SQL'})
...
box.execute("INSERT INTO \"test\" VALUES(1, 2);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: conflict'
+- error: 'Failed to execute SQL statement: Check constraint failed ''conflict'': X>10'
...
box.execute("INSERT INTO \"test\" VALUES(11, 11);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: physics'
+- error: 'Failed to execute SQL statement: Check constraint failed ''physics'': X<Y'
...
box.execute("INSERT INTO \"test\" VALUES(12, 11);")
---
@@ -255,6 +277,183 @@ box.execute("CREATE TABLE t5(x INT PRIMARY KEY, y INT, CHECK( x*y < ? ));")
- error: 'Failed to create check constraint ''CK_CONSTRAINT_1_T5'': bindings are not
allowed in DDL'
...
+--
+-- Test ck constraint corner cases
+--
+s = box.schema.create_space('test', {engine = engine})
+---
+...
+_ = s:create_index('pk')
+---
+...
+s:format({{name='X', type='any'}, {name='Y', type='integer'}, {name='Z', type='integer', is_nullable=true}})
+---
+...
+ck_not_null = box.space._ck_constraint:insert({'ZnotNULL', s.id, false, 'Z IS NOT NULL', 'SQL'})
+---
+...
+s:insert({1, 2, box.NULL})
+---
+- error: 'Check constraint failed ''ZnotNULL'': Z IS NOT NULL'
+...
+s:insert({1, 2})
+---
+- error: 'Check constraint failed ''ZnotNULL'': Z IS NOT NULL'
+...
+_ = box.space._ck_constraint:delete({'ZnotNULL', s.id})
+---
+...
+_ = box.space._ck_constraint:insert({'XlessY', s.id, false, 'X < Y and Y < Z', 'SQL'})
+---
+...
+s:insert({'1', 2})
+---
+- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+...
+s:insert({})
+---
+- error: Tuple field 1 required by space format is missing
+...
+s:insert({2, 1})
+---
+- error: 'Check constraint failed ''XlessY'': X < Y and Y < Z'
+...
+s:insert({1, 2})
+---
+- [1, 2]
+...
+s:insert({2, 3, 1})
+---
+- error: 'Check constraint failed ''XlessY'': X < Y and Y < Z'
+...
+s:insert({2, 3, 4})
+---
+- [2, 3, 4]
+...
+s:update({2}, {{'+', 2, 3}})
+---
+- error: 'Check constraint failed ''XlessY'': X < Y and Y < Z'
+...
+s:update({2}, {{'+', 2, 3}, {'+', 3, 3}})
+---
+- [2, 6, 7]
+...
+s:replace({2, 1, 3})
+---
+- error: 'Check constraint failed ''XlessY'': X < Y and Y < Z'
+...
+box.snapshot()
+---
+- ok
+...
+s = box.space["test"]
+---
+...
+s:update({2}, {{'+', 2, 3}})
+---
+- error: 'Check constraint failed ''XlessY'': X < Y and Y < Z'
+...
+s:update({2}, {{'+', 2, 3}, {'+', 3, 3}})
+---
+- [2, 9, 10]
+...
+s:replace({2, 1, 3})
+---
+- error: 'Check constraint failed ''XlessY'': X < Y and Y < Z'
+...
+s:drop()
+---
+...
+--
+-- Test complex CHECK constraints.
+--
+s = box.schema.create_space('test', {engine = engine})
+---
+...
+s:format({{name='X', type='integer'}, {name='Y', type='integer'}, {name='Z', type='integer'}})
+---
+...
+_ = s:create_index('pk', {parts = {3, 'integer'}})
+---
+...
+_ = s:create_index('unique', {parts = {1, 'integer'}})
+---
+...
+_ = box.space._ck_constraint:insert({'complex1', s.id, false, 'x+y==11 OR x*y==12 OR x/y BETWEEN 5 AND 8 OR -x == y+10', 'SQL'})
+---
+...
+s:insert({1, 10, 1})
+---
+- [1, 10, 1]
+...
+s:update({1}, {{'=', 1, 4}, {'=', 2, 3}})
+---
+- [4, 3, 1]
+...
+s:update({1}, {{'=', 1, 12}, {'=', 2, 2}})
+---
+- [12, 2, 1]
+...
+s:update({1}, {{'=', 1, 12}, {'=', 2, -22}})
+---
+- [12, -22, 1]
+...
+s:update({1}, {{'=', 1, 0}, {'=', 2, 1}})
+---
+- error: 'Check constraint failed ''complex1'': x+y==11 OR x*y==12 OR x/y BETWEEN
+ 5 AND 8 OR -x == y+10'
+...
+s:get({1})
+---
+- [12, -22, 1]
+...
+s:update({1}, {{'=', 1, 0}, {'=', 2, 2}})
+---
+- error: 'Check constraint failed ''complex1'': x+y==11 OR x*y==12 OR x/y BETWEEN
+ 5 AND 8 OR -x == y+10'
+...
+s:get({1})
+---
+- [12, -22, 1]
+...
+s:drop()
+---
+...
+s = box.schema.create_space('test', {engine = engine})
+---
+...
+s:format({{name='X', type='integer'}, {name='Z', type='any'}})
+---
+...
+_ = s:create_index('pk', {parts = {1, 'integer'}})
+---
+...
+_ = box.space._ck_constraint:insert({'complex2', s.id, false, 'typeof(coalesce(z,0))==\'integer\'', 'SQL'})
+---
+...
+s:insert({1, 'string'})
+---
+- error: 'Check constraint failed ''complex2'': typeof(coalesce(z,0))==''integer'''
+...
+s:insert({1, {map=true}})
+---
+- error: 'Check constraint failed ''complex2'': typeof(coalesce(z,0))==''integer'''
+...
+s:insert({1, {'a', 'r','r','a','y'}})
+---
+- error: 'Check constraint failed ''complex2'': typeof(coalesce(z,0))==''integer'''
+...
+s:insert({1, 3.14})
+---
+- error: 'Check constraint failed ''complex2'': typeof(coalesce(z,0))==''integer'''
+...
+s:insert({1, 666})
+---
+- [1, 666]
+...
+s:drop()
+---
+...
test_run:cmd("clear filter")
---
- true
diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua
index a6b0efcf0..a284edf52 100644
--- a/test/sql/checks.test.lua
+++ b/test/sql/checks.test.lua
@@ -33,9 +33,11 @@ box.space._ck_constraint:insert({'CK_CONSTRAINT_01', 513, false, 'X<5', 'SQL'})
box.space._ck_constraint:count({})
box.execute("INSERT INTO \"test\" VALUES(5);")
+box.space.test:insert({5})
box.space._ck_constraint:replace({'CK_CONSTRAINT_01', 513, false, 'X<=5', 'SQL'})
box.execute("INSERT INTO \"test\" VALUES(5);")
box.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()
@@ -47,8 +49,11 @@ box.space._space:delete({513})
box.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.execute("INSERT INTO t1 VALUES (7, 1, 1)")
+box.space.T1:insert({7, 1, 1})
box.execute("INSERT INTO t1 VALUES (2, 1, 1)")
+box.space.T1:insert({2, 1, 1})
box.execute("INSERT INTO t1 VALUES (2, 4, 1)")
+box.space.T1:update({1}, {{'+', 1, 5}})
box.execute("DROP TABLE t1")
-- Test space creation rollback on spell error in ck constraint.
@@ -95,4 +100,60 @@ box.execute("DROP TABLE w2;")
--
box.execute("CREATE TABLE t5(x INT PRIMARY KEY, y INT, CHECK( x*y < ? ));")
+--
+-- Test ck constraint corner cases
+--
+s = box.schema.create_space('test', {engine = engine})
+_ = s:create_index('pk')
+s:format({{name='X', type='any'}, {name='Y', type='integer'}, {name='Z', type='integer', is_nullable=true}})
+ck_not_null = box.space._ck_constraint:insert({'ZnotNULL', s.id, false, 'Z IS NOT NULL', 'SQL'})
+s:insert({1, 2, box.NULL})
+s:insert({1, 2})
+_ = box.space._ck_constraint:delete({'ZnotNULL', s.id})
+_ = box.space._ck_constraint:insert({'XlessY', s.id, false, 'X < Y and Y < Z', 'SQL'})
+s:insert({'1', 2})
+s:insert({})
+s:insert({2, 1})
+s:insert({1, 2})
+s:insert({2, 3, 1})
+s:insert({2, 3, 4})
+s:update({2}, {{'+', 2, 3}})
+s:update({2}, {{'+', 2, 3}, {'+', 3, 3}})
+s:replace({2, 1, 3})
+box.snapshot()
+s = box.space["test"]
+s:update({2}, {{'+', 2, 3}})
+s:update({2}, {{'+', 2, 3}, {'+', 3, 3}})
+s:replace({2, 1, 3})
+s:drop()
+
+--
+-- Test complex CHECK constraints.
+--
+s = box.schema.create_space('test', {engine = engine})
+s:format({{name='X', type='integer'}, {name='Y', type='integer'}, {name='Z', type='integer'}})
+_ = s:create_index('pk', {parts = {3, 'integer'}})
+_ = s:create_index('unique', {parts = {1, 'integer'}})
+_ = box.space._ck_constraint:insert({'complex1', s.id, false, 'x+y==11 OR x*y==12 OR x/y BETWEEN 5 AND 8 OR -x == y+10', 'SQL'})
+s:insert({1, 10, 1})
+s:update({1}, {{'=', 1, 4}, {'=', 2, 3}})
+s:update({1}, {{'=', 1, 12}, {'=', 2, 2}})
+s:update({1}, {{'=', 1, 12}, {'=', 2, -22}})
+s:update({1}, {{'=', 1, 0}, {'=', 2, 1}})
+s:get({1})
+s:update({1}, {{'=', 1, 0}, {'=', 2, 2}})
+s:get({1})
+s:drop()
+
+s = box.schema.create_space('test', {engine = engine})
+s:format({{name='X', type='integer'}, {name='Z', type='any'}})
+_ = s:create_index('pk', {parts = {1, 'integer'}})
+_ = box.space._ck_constraint:insert({'complex2', s.id, false, 'typeof(coalesce(z,0))==\'integer\'', 'SQL'})
+s:insert({1, 'string'})
+s:insert({1, {map=true}})
+s:insert({1, {'a', 'r','r','a','y'}})
+s:insert({1, 3.14})
+s:insert({1, 666})
+s:drop()
+
test_run:cmd("clear filter")
diff --git a/test/sql/errinj.result b/test/sql/errinj.result
index e5bcbc9db..59abb588f 100644
--- a/test/sql/errinj.result
+++ b/test/sql/errinj.result
@@ -490,7 +490,8 @@ _ = box.space._ck_constraint:insert({'CK_CONSTRAINT_01', s.id, false, 'X<5', 'SQ
...
box.execute("INSERT INTO \"test\" VALUES(5);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_01'
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_01'':
+ X<5'
...
errinj.set("ERRINJ_WAL_IO", true)
---
@@ -521,7 +522,8 @@ _ = box.space._ck_constraint:delete({'CK_CONSTRAINT_01', s.id})
...
box.execute("INSERT INTO \"test\" VALUES(6);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_01'
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_01'':
+ X<=5'
...
errinj.set("ERRINJ_WAL_IO", false)
---
@@ -557,11 +559,13 @@ _ = box.space._ck_constraint:insert({'Xgreater10', s.id, false, 'X > 10', 'SQL'}
...
box.execute("INSERT INTO \"test\" VALUES(2, 1);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: Xgreater10'
+- error: 'Failed to execute SQL statement: Check constraint failed ''Xgreater10'':
+ X > 10'
...
box.execute("INSERT INTO \"test\" VALUES(20, 10);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: XlessY'
+- error: 'Failed to execute SQL statement: Check constraint failed ''XlessY'': X <
+ Y'
...
box.execute("INSERT INTO \"test\" VALUES(20, 100);")
---
@@ -584,11 +588,13 @@ errinj.set("ERRINJ_WAL_IO", false)
...
box.execute("INSERT INTO \"test\" VALUES(2, 1);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: XlessY'
+- error: 'Failed to execute SQL statement: Check constraint failed ''XlessY'': X <
+ Y'
...
box.execute("INSERT INTO \"test\" VALUES(20, 10);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: XlessY'
+- error: 'Failed to execute SQL statement: Check constraint failed ''XlessY'': X <
+ Y'
...
box.execute("INSERT INTO \"test\" VALUES(20, 100);")
---
diff --git a/test/sql/gh-2981-check-autoinc.result b/test/sql/gh-2981-check-autoinc.result
index 7384c81e8..e57789897 100644
--- a/test/sql/gh-2981-check-autoinc.result
+++ b/test/sql/gh-2981-check-autoinc.result
@@ -29,7 +29,8 @@ box.execute("insert into t1 values (18, null);")
...
box.execute("insert into t1(s2) values (null);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1'
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_1_T1'':
+ s1 <> 19'
...
box.execute("insert into t2 values (18, null);")
---
@@ -37,7 +38,8 @@ box.execute("insert into t2 values (18, null);")
...
box.execute("insert into t2(s2) values (null);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T2'
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_1_T2'':
+ s1 <> 19 AND s1 <> 25'
...
box.execute("insert into t2 values (24, null);")
---
@@ -45,7 +47,8 @@ box.execute("insert into t2 values (24, null);")
...
box.execute("insert into t2(s2) values (null);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T2'
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_1_T2'':
+ s1 <> 19 AND s1 <> 25'
...
box.execute("insert into t3 values (9, null)")
---
@@ -53,7 +56,8 @@ box.execute("insert into t3 values (9, null)")
...
box.execute("insert into t3(s2) values (null)")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T3'
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_1_T3'':
+ s1 < 10'
...
box.execute("DROP TABLE t1")
---
diff --git a/test/sql/types.result b/test/sql/types.result
index 582785413..ccbdd8c50 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -709,7 +709,8 @@ box.execute("CREATE TABLE t1 (id INT PRIMARY KEY, a BOOLEAN CHECK (a = true));")
...
box.execute("INSERT INTO t1 VALUES (1, false);")
---
-- error: 'Failed to execute SQL statement: CHECK constraint failed: CK_CONSTRAINT_1_T1'
+- error: 'Failed to execute SQL statement: Check constraint failed ''CK_CONSTRAINT_1_T1'':
+ a = true'
...
box.execute("INSERT INTO t1 VALUES (1, true);")
---
--
2.21.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] [PATCH v4 3/4] box: introduce column_mask for ck constraint
2019-05-16 13:56 [tarantool-patches] [PATCH v4 0/4] box: run checks on insertions in LUA spaces Kirill Shcherbatov
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 1/4] schema: add new system space for CHECK constraints Kirill Shcherbatov
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 2/4] box: run check constraint tests on space alter Kirill Shcherbatov
@ 2019-05-16 13:56 ` Kirill Shcherbatov
2019-05-19 16:02 ` [tarantool-patches] " Vladislav Shpilevoy
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 4/4] box: user-friendly interface to manage ck constraints Kirill Shcherbatov
3 siblings, 1 reply; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-16 13:56 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
This patch introduces a smart bitmask of fields are mentioned in
ck constraint. This allows to do not bind useless fields and
do not parse more fields that is required.
Needed for #3691
---
src/box/ck_constraint.c | 66 ++++++++++++++++++++++++++++++++++------
src/box/ck_constraint.h | 6 ++++
src/box/sql.h | 5 ++-
src/box/sql/build.c | 2 +-
src/box/sql/resolve.c | 19 +++++++++---
src/box/sql/sqlInt.h | 6 ++++
test/sql/checks.result | 59 +++++++++++++++++++++++++++++++++++
test/sql/checks.test.lua | 20 ++++++++++++
8 files changed, 167 insertions(+), 16 deletions(-)
diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c
index 43798be76..4756f7970 100644
--- a/src/box/ck_constraint.c
+++ b/src/box/ck_constraint.c
@@ -46,17 +46,21 @@ const char *ck_constraint_language_strs[] = {"SQL"};
* tree traversal.
* @param ck_constraint Check constraint object to update.
* @param space_def Space definition to use.
+ * @param column_mask[out] The "smart" column mask of fields are
+ * referenced by AST.
* @retval 0 on success.
* @retval -1 on error.
*/
static int
ck_constraint_resolve_field_names(struct Expr *expr,
- struct space_def *space_def)
+ struct space_def *space_def,
+ uint64_t *column_mask)
{
struct Parse parser;
sql_parser_create(&parser, sql_get(), default_flags);
parser.parse_only = true;
- sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL);
+ sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL,
+ column_mask);
int rc = parser.is_aborted ? -1 : 0;
sql_parser_destroy(&parser);
return rc;
@@ -75,6 +79,8 @@ ck_constraint_resolve_field_names(struct Expr *expr,
* given @ck_constraint_def, see for
* (sql_expr_compile +
* ck_constraint_resolve_space_def) implementation.
+ * @param column_mask The "smart" column mask of fields are
+ * referenced by @a expr.
* @param space_def The space definition of the space this check
* constraint is constructed for.
* @retval not NULL sql_stmt program pointer on success.
@@ -82,7 +88,8 @@ ck_constraint_resolve_field_names(struct Expr *expr,
*/
static struct sql_stmt *
ck_constraint_program_compile(struct ck_constraint_def *ck_constraint_def,
- struct Expr *expr, struct space_def *space_def)
+ struct Expr *expr, uint64_t column_mask,
+ struct space_def *space_def)
{
struct sql *db = sql_get();
struct Parse parser;
@@ -98,10 +105,24 @@ ck_constraint_program_compile(struct ck_constraint_def *ck_constraint_def,
* bind tuple fields there before execution.
*/
uint32_t field_count = space_def->field_count;
+ /*
+ * Use column mask to prepare binding only for
+ * referenced fields.
+ */
int bind_tuple_reg = sqlGetTempRange(&parser, field_count);
- for (uint32_t i = 0; i < field_count; i++) {
+ struct bit_iterator it;
+ bit_iterator_init(&it, &column_mask, sizeof(uint64_t), true);
+ size_t used_fieldno = bit_iterator_next(&it);
+ for (; used_fieldno < SIZE_MAX; used_fieldno = bit_iterator_next(&it)) {
sqlVdbeAddOp2(v, OP_Variable, ++parser.nVar,
- bind_tuple_reg + i);
+ bind_tuple_reg + used_fieldno);
+ }
+ if (column_mask_fieldno_is_set(column_mask, 63)) {
+ used_fieldno = 64;
+ for (; used_fieldno < (size_t)field_count; used_fieldno++) {
+ sqlVdbeAddOp2(v, OP_Variable, ++parser.nVar,
+ bind_tuple_reg + used_fieldno);
+ }
}
/* Generate ck constraint test code. */
vdbe_emit_ck_constraint(&parser, expr, ck_constraint_def->name,
@@ -142,6 +163,13 @@ ck_constraint_program_run(struct ck_constraint *ck_constraint,
*/
struct space *space = space_by_id(ck_constraint->space_id);
assert(space != NULL);
+ /* Use column mask to bind only referenced fields. */
+ struct bit_iterator it;
+ bit_iterator_init(&it, &ck_constraint->column_mask,
+ sizeof(uint64_t), true);
+ size_t used_fieldno = bit_iterator_next(&it);
+ if (used_fieldno == SIZE_MAX)
+ return 0;
/*
* When last format fields are nullable, they are
* 'optional' i.e. they may not be present in the tuple.
@@ -149,15 +177,33 @@ ck_constraint_program_run(struct ck_constraint *ck_constraint,
uint32_t tuple_field_count = mp_decode_array(&new_tuple);
uint32_t field_count =
MIN(tuple_field_count, space->def->field_count);
- for (uint32_t i = 0; i < field_count; i++) {
+ uint32_t bind_pos = 1;
+ for (uint32_t fieldno = 0; fieldno < field_count; fieldno++) {
+ if (used_fieldno == SIZE_MAX &&
+ !column_mask_fieldno_is_set(ck_constraint->column_mask,
+ 63)) {
+ /* No more required fields are left. */
+ break;
+ }
+ if (used_fieldno != SIZE_MAX &&
+ (size_t)fieldno < used_fieldno) {
+ /*
+ * Skip unused fields is mentioned in
+ * column mask.
+ */
+ mp_next(&new_tuple);
+ continue;
+ }
struct sql_bind bind;
- if (sql_bind_decode(&bind, i + 1, &new_tuple) != 0 ||
- sql_bind_column(ck_constraint->stmt, &bind, i + 1) != 0) {
+ if (sql_bind_decode(&bind, bind_pos, &new_tuple) != 0 ||
+ sql_bind_column(ck_constraint->stmt, &bind, bind_pos) != 0) {
diag_set(ClientError, ER_CK_CONSTRAINT_FAILED,
ck_constraint->def->name,
ck_constraint->def->expr_str);
return -1;
}
+ bind_pos++;
+ used_fieldno = bit_iterator_next(&it);
}
/* Checks VDBE can't expire, reset expired flag and go. */
struct Vdbe *v = (struct Vdbe *) ck_constraint->stmt;
@@ -220,7 +266,8 @@ ck_constraint_new(struct ck_constraint_def *ck_constraint_def,
sql_expr_compile(sql_get(), ck_constraint_def->expr_str,
strlen(ck_constraint_def->expr_str));
if (expr == NULL ||
- ck_constraint_resolve_field_names(expr, space_def) != 0) {
+ ck_constraint_resolve_field_names(expr, space_def,
+ &ck_constraint->column_mask) != 0) {
diag_set(ClientError, ER_CREATE_CK_CONSTRAINT,
ck_constraint_def->name,
box_error_message(box_error_last()));
@@ -228,6 +275,7 @@ ck_constraint_new(struct ck_constraint_def *ck_constraint_def,
}
ck_constraint->stmt =
ck_constraint_program_compile(ck_constraint_def, expr,
+ ck_constraint->column_mask,
space_def);
if (ck_constraint->stmt == NULL)
goto error;
diff --git a/src/box/ck_constraint.h b/src/box/ck_constraint.h
index a948a074e..141bdc11d 100644
--- a/src/box/ck_constraint.h
+++ b/src/box/ck_constraint.h
@@ -88,6 +88,12 @@ struct ck_constraint {
* message when ck condition unsatisfied.
*/
struct sql_stmt *stmt;
+ /**
+ * The "smart" column mask of fields are referenced by
+ * AST. Then the last bit of the mask is set when field
+ * #63 and greater has referenced.
+ */
+ uint64_t column_mask;
/**
* Trigger object executing check constraint before
* insert and replace operations.
diff --git a/src/box/sql.h b/src/box/sql.h
index 9c8a96a75..13f5984f0 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -279,11 +279,14 @@ sql_expr_list_append(struct sql *db, struct ExprList *expr_list,
* @param type NC_IsCheck or NC_IdxExpr.
* @param expr Expression to resolve. May be NULL.
* @param expr_list Expression list to resolve. May be NUL.
+ * @param[out] column_mask The "smart" column mask of fields are
+ * referenced by AST.
*/
void
sql_resolve_self_reference(struct Parse *parser, struct space_def *def,
int type, struct Expr *expr,
- struct ExprList *expr_list);
+ struct ExprList *expr_list,
+ uint64_t *column_mask);
/**
* Initialize a new parser object.
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index f2bbbb9c4..5eac4ee40 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2198,7 +2198,7 @@ index_fill_def(struct Parse *parse, struct index *index,
for (int i = 0; i < expr_list->nExpr; i++) {
struct Expr *expr = expr_list->a[i].pExpr;
sql_resolve_self_reference(parse, space_def, NC_IdxExpr,
- expr, 0);
+ expr, NULL, NULL);
if (parse->is_aborted)
goto cleanup;
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 7cf79692c..cbd5cd724 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -556,8 +556,11 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
case TK_ID:{
if ((pNC->ncFlags & NC_AllowAgg) != 0)
pNC->ncFlags |= NC_HasUnaggregatedId;
- return lookupName(pParse, 0, pExpr->u.zToken, pNC,
- pExpr);
+ int rc = lookupName(pParse, 0, pExpr->u.zToken, pNC,
+ pExpr);
+ column_mask_set_fieldno(&pNC->column_mask,
+ pExpr->iColumn);
+ return rc;
}
/* A table name and column name: ID.ID
@@ -583,8 +586,11 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
zTable = pRight->pLeft->u.zToken;
zColumn = pRight->pRight->u.zToken;
}
- return lookupName(pParse, zTable, zColumn, pNC,
- pExpr);
+ int rc = lookupName(pParse, zTable, zColumn, pNC,
+ pExpr);
+ column_mask_set_fieldno(&pNC->column_mask,
+ pExpr->iColumn);
+ return rc;
}
/* Resolve function names
@@ -1612,7 +1618,7 @@ sqlResolveSelectNames(Parse * pParse, /* The parser context */
void
sql_resolve_self_reference(struct Parse *parser, struct space_def *def,
int type, struct Expr *expr,
- struct ExprList *expr_list)
+ struct ExprList *expr_list, uint64_t *column_mask)
{
/* Fake SrcList for parser->create_table_def */
SrcList sSrc;
@@ -1632,8 +1638,11 @@ sql_resolve_self_reference(struct Parse *parser, struct space_def *def,
sNC.pParse = parser;
sNC.pSrcList = &sSrc;
sNC.ncFlags = type;
+ sNC.column_mask = 0;
if (sqlResolveExprNames(&sNC, expr) != 0)
return;
if (expr_list != NULL)
sqlResolveExprListNames(&sNC, expr_list);
+ if (column_mask != NULL)
+ *column_mask = sNC.column_mask;
}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index d353d7906..0eca37653 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2349,6 +2349,12 @@ struct NameContext {
int nRef; /* Number of names resolved by this context */
int nErr; /* Number of errors encountered while resolving names */
u16 ncFlags; /* Zero or more NC_* flags defined below */
+ /**
+ * The "smart" column mask of fields are referenced
+ * by AST. Then the last bit of the mask is set when
+ * field #63 and greater has referenced.
+ */
+ uint64_t column_mask;
};
/*
diff --git a/test/sql/checks.result b/test/sql/checks.result
index b74572e3e..04f58c90d 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -454,6 +454,65 @@ s:insert({1, 666})
s:drop()
---
...
+--
+-- Test binding optimisation.
+--
+s = box.schema.create_space('test')
+---
+...
+_ = s:create_index('pk', {parts = {1, 'integer'}})
+---
+...
+format65 = {}
+---
+...
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+for i = 1,66 do
+ table.insert(format65, {name='X'..i, type='integer', is_nullable = true})
+end
+test_run:cmd("setopt delimiter ''");
+---
+...
+s:format(format65)
+---
+...
+_ = box.space._ck_constraint:insert({'X1is666andX65is666', s.id, false, 'X1 == 666 and X65 == 666 and X63 IS NOT NULL', 'SQL'})
+---
+...
+s:insert(s:frommap({X1 = 1, X65 = 1}))
+---
+- error: 'Check constraint failed ''X1is666andX65is666'': X1 == 666 and X65 == 666
+ and X63 IS NOT NULL'
+...
+s:insert(s:frommap({X1 = 666, X65 = 1}))
+---
+- error: 'Check constraint failed ''X1is666andX65is666'': X1 == 666 and X65 == 666
+ and X63 IS NOT NULL'
+...
+s:insert(s:frommap({X1 = 1, X65 = 666}))
+---
+- error: 'Check constraint failed ''X1is666andX65is666'': X1 == 666 and X65 == 666
+ and X63 IS NOT NULL'
+...
+s:insert(s:frommap({X1 = 666, X65 = 666}))
+---
+- error: 'Check constraint failed ''X1is666andX65is666'': X1 == 666 and X65 == 666
+ and X63 IS NOT NULL'
+...
+s:insert(s:frommap({X1 = 666, X65 = 666, X63 = 1}))
+---
+- [666, null, null, null, null, null, null, null, null, null, null, null, null, null,
+ null, null, null, null, null, null, null, null, null, null, null, null, null, null,
+ null, null, null, null, null, null, null, null, null, null, null, null, null, null,
+ null, null, null, null, null, null, null, null, null, null, null, null, null, null,
+ null, null, null, null, null, null, 1, null, 666]
+...
+s:drop()
+---
+...
test_run:cmd("clear filter")
---
- true
diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua
index a284edf52..a23dcce51 100644
--- a/test/sql/checks.test.lua
+++ b/test/sql/checks.test.lua
@@ -156,4 +156,24 @@ s:insert({1, 3.14})
s:insert({1, 666})
s:drop()
+--
+-- Test binding optimisation.
+--
+s = box.schema.create_space('test')
+_ = s:create_index('pk', {parts = {1, 'integer'}})
+format65 = {}
+test_run:cmd("setopt delimiter ';'")
+for i = 1,66 do
+ table.insert(format65, {name='X'..i, type='integer', is_nullable = true})
+end
+test_run:cmd("setopt delimiter ''");
+s:format(format65)
+_ = box.space._ck_constraint:insert({'X1is666andX65is666', s.id, false, 'X1 == 666 and X65 == 666 and X63 IS NOT NULL', 'SQL'})
+s:insert(s:frommap({X1 = 1, X65 = 1}))
+s:insert(s:frommap({X1 = 666, X65 = 1}))
+s:insert(s:frommap({X1 = 1, X65 = 666}))
+s:insert(s:frommap({X1 = 666, X65 = 666}))
+s:insert(s:frommap({X1 = 666, X65 = 666, X63 = 1}))
+s:drop()
+
test_run:cmd("clear filter")
--
2.21.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] [PATCH v4 4/4] box: user-friendly interface to manage ck constraints
2019-05-16 13:56 [tarantool-patches] [PATCH v4 0/4] box: run checks on insertions in LUA spaces Kirill Shcherbatov
` (2 preceding siblings ...)
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 3/4] box: introduce column_mask for ck constraint Kirill Shcherbatov
@ 2019-05-16 13:56 ` Kirill Shcherbatov
2019-05-19 16:02 ` [tarantool-patches] " Vladislav Shpilevoy
3 siblings, 1 reply; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-16 13:56 UTC (permalink / raw)
To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov
Closes #3691
@TarantoolBot document
Title: check constraint for Lua space
The check constraint is a type of integrity constraint which
specifies a requirement that must be met by tuple before it
is inserted into space. The constraint result must be predictable.
Expression in check constraint must be <boolean value expression>
I.e. return boolean result.
Now it is possible to create ck constraints only for empty space
having format. Constraint expression is a string that defines
relations between top-level tuple fields.
Take into account that all names are converted to an uppercase
before resolve(like SQL does), use \" sign for names of fields
that were created not with SQL.
The check constraints are fired on insertion to the Lua space
together with Lua space triggers. The execution order of
ck constraints checks and space triggers follows their creation
sequence.
Note: this patch changes the CK constraints execution order for
SQL. Previously check of CK constraints integrity was fired before
tuple is formed; meanwhile now they are implemented as NoSQL before
replace triggers, which are fired right before tuple insertion.
In turn, type casts are performed earlier than msgpack
serialization. You should be careful with functions that use
field types in your check constrains (like typeof()).
Consider following situation:
```
box.execute("CREATE TABLE t2(id INT primary key,
x INTEGER CHECK (x > 1));")
box.execute("INSERT INTO t2 VALUES(3, 1.1)")
```
the last operation would fail because 1.1 is silently
cast to integer 1 which is not greater than 1.
To create a new CK constraint for a space, use
space:create_check_constraint method. All space constraints are
shown in space.ck_constraint table. To drop ck constraint,
use :drop method.
Example:
```
s1 = box.schema.create_space('test1')
pk = s1:create_index('pk')
ck = s1:create_check_constraint('physics', 'X < Y')
s1:insert({2, 1}) -- fail
ck:drop()
```
---
src/box/alter.cc | 2 +
src/box/lua/schema.lua | 35 +++++++++++++++-
src/box/lua/space.cc | 63 ++++++++++++++++++++++++++++
test/sql/checks.result | 90 ++++++++++++++++++++++++++++++++++++++++
test/sql/checks.test.lua | 29 +++++++++++++
5 files changed, 218 insertions(+), 1 deletion(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 746ce62f6..ab6b1d5f8 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -4237,6 +4237,8 @@ on_replace_ck_constraint_commit(struct trigger *trigger, void *event)
{
struct txn_stmt *stmt = txn_last_stmt((struct txn *) event);
struct ck_constraint *ck = (struct ck_constraint *)trigger->data;
+ struct space *space = space_by_id(ck->space_id);
+ trigger_run_xc(&on_alter_space, space);
if (stmt->old_tuple != NULL)
ck_constraint_delete(ck);
}
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index e01f500e6..a30d6aa5b 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1185,6 +1185,15 @@ local function check_primary_index(space)
end
box.internal.check_primary_index = check_primary_index -- for net.box
+-- Helper function to check ck_constraint:method() usage
+local function check_ck_constraint_arg(ck_constraint, method)
+ if type(ck_constraint) ~= 'table' or ck_constraint.name == nil then
+ local fmt = 'Use ck_constraint:%s(...) instead of ck_constraint.%s(...)'
+ error(string.format(fmt, method, method))
+ end
+end
+box.internal.check_ck_constraint_arg = check_ck_constraint_arg
+
box.internal.schema_version = builtin.box_schema_version
local function check_iterator_type(opts, key_is_nil)
@@ -1533,7 +1542,16 @@ space_mt.auto_increment = function(space, tuple)
table.insert(tuple, 1, max + 1)
return space:insert(tuple)
end
-
+-- Manage space ck constraints
+space_mt.create_check_constraint = function(space, name, expr_str)
+ check_space_arg(space, 'create_constraint')
+ if name == nil or expr_str == nil then
+ box.error(box.error.PROC_LUA,
+ "Usage: space:create_constraint(name, expr_str)")
+ end
+ box.space._ck_constraint:insert({name, space.id, false, expr_str, 'SQL'})
+ return space.ck_constraint[name]
+end
space_mt.pairs = function(space, key, opts)
check_space_arg(space, 'pairs')
local pk = space.index[0]
@@ -1579,10 +1597,17 @@ end
space_mt.frommap = box.internal.space.frommap
space_mt.__index = space_mt
+local ck_constraint_mt = {}
+ck_constraint_mt.drop = function(ck_constraint)
+ check_ck_constraint_arg(ck_constraint, 'drop')
+ box.space._ck_constraint:delete({ck_constraint.name, ck_constraint.space_id})
+end
+
box.schema.index_mt = base_index_mt
box.schema.memtx_index_mt = memtx_index_mt
box.schema.vinyl_index_mt = vinyl_index_mt
box.schema.space_mt = space_mt
+box.schema.ck_constraint_mt = ck_constraint_mt
--
-- Wrap a global space/index metatable into a space/index local
@@ -1628,6 +1653,14 @@ function box.schema.space.bless(space)
end
end
end
+ if type(space.ck_constraint) == 'table' and space.enabled then
+ for j, ck_constraint in pairs(space.ck_constraint) do
+ if type(j) == 'string' then
+ setmetatable(ck_constraint,
+ wrap_schema_object_mt('ck_constraint_mt'))
+ end
+ end
+ end
end
local sequence_mt = {}
diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index ce4287bba..9cec1cbde 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -28,6 +28,7 @@
* THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
+#include "box/ck_constraint.h"
#include "box/lua/space.h"
#include "box/lua/tuple.h"
#include "box/lua/key_def.h"
@@ -147,6 +148,66 @@ lbox_space_before_replace(struct lua_State *L)
lbox_push_txn_stmt, lbox_pop_txn_stmt);
}
+/**
+ * Make ck_constraints available in Lua, via ck_constraint[]
+ * array.
+ * Returns a new table representing a space on top of the Lua
+ * stack.
+ */
+static void
+lbox_ck_constraint(struct lua_State *L, struct space *space, int i)
+{
+ lua_getfield(L, i, "ck_constraint");
+ if (lua_isnil(L, -1)) {
+ lua_pop(L, 1);
+ lua_pushstring(L, "ck_constraint");
+ lua_newtable(L);
+ lua_settable(L, i);
+ lua_getfield(L, i, "ck_constraint");
+ } else {
+ lua_pushnil(L);
+ while (lua_next(L, -2) != 0) {
+ size_t name_len;
+ const char *name = lua_tolstring(L, -2, &name_len);
+ /*
+ * Remove ck_constraint only if it was
+ * deleted.
+ */
+ if (space_ck_constraint_by_name(space, name,
+ (uint32_t)name_len) == NULL) {
+ lua_pushlstring(L, name, name_len);
+ lua_pushnil(L);
+ lua_settable(L, -5);
+ }
+ lua_pop(L, 1);
+ }
+ }
+ struct ck_constraint *ck_constraint = NULL;
+ rlist_foreach_entry(ck_constraint, &space->ck_constraint, link) {
+ lua_getfield(L, i, ck_constraint->def->name);
+ if (lua_isnil(L, -1)) {
+ lua_pop(L, 1);
+ lua_pushstring(L, ck_constraint->def->name);
+ lua_newtable(L);
+ lua_settable(L, -3);
+ lua_getfield(L, -1, ck_constraint->def->name);
+ assert(!lua_isnil(L, -1));
+ }
+
+ lua_pushstring(L, ck_constraint->def->name);
+ lua_setfield(L, -2, "name");
+
+ lua_pushnumber(L, space->def->id);
+ lua_setfield(L, -2, "space_id");
+
+ lua_pushstring(L, ck_constraint->def->expr_str);
+ lua_setfield(L, -2, "expr_str");
+
+ lua_setfield(L, -2, ck_constraint->def->name);
+ }
+ lua_pop(L, 1);
+}
+
/**
* Make a single space available in Lua,
* via box.space[] array.
@@ -337,6 +398,8 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i)
lua_pop(L, 1); /* pop the index field */
+ lbox_ck_constraint(L, space, i);
+
lua_getfield(L, LUA_GLOBALSINDEX, "box");
lua_pushstring(L, "schema");
lua_gettable(L, -2);
diff --git a/test/sql/checks.result b/test/sql/checks.result
index 04f58c90d..d87106c42 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -513,6 +513,96 @@ s:insert(s:frommap({X1 = 666, X65 = 666, X63 = 1}))
s:drop()
---
...
+--
+-- Test ck constraints LUA integration.
+--
+s1 = box.schema.create_space('test1')
+---
+...
+_ = s1:create_index('pk')
+---
+...
+s1:format({{name='X', type='any'}, {name='Y', type='integer'}})
+---
+...
+s2 = box.schema.create_space('test2')
+---
+...
+_ = s2:create_index('pk')
+---
+...
+s2:format({{name='X', type='any'}, {name='Y', type='integer'}})
+---
+...
+_ = s1:create_check_constraint('physics', 'X < Y')
+---
+...
+_ = s1:create_check_constraint('greater', 'X > 20')
+---
+...
+_ = s2:create_check_constraint('physics', 'X > Y')
+---
+...
+_ = s2:create_check_constraint('greater', 'X > 20')
+---
+...
+s1.ck_constraint.physics ~= nil
+---
+- true
+...
+s1.ck_constraint.greater ~= nil
+---
+- true
+...
+s2.ck_constraint.physics ~= nil
+---
+- true
+...
+s2.ck_constraint.greater ~= nil
+---
+- true
+...
+s1:insert({2, 1})
+---
+- error: 'Check constraint failed ''greater'': X > 20'
+...
+s1:insert({21, 20})
+---
+- error: 'Check constraint failed ''physics'': X < Y'
+...
+s2:insert({1, 2})
+---
+- error: 'Check constraint failed ''greater'': X > 20'
+...
+s2:insert({21, 22})
+---
+- error: 'Check constraint failed ''physics'': X > Y'
+...
+s2.ck_constraint.greater:drop()
+---
+...
+s2.ck_constraint.physics ~= nil
+---
+- true
+...
+s2.ck_constraint.greater == nil
+---
+- true
+...
+s1:insert({2, 1})
+---
+- error: 'Check constraint failed ''greater'': X > 20'
+...
+s2:insert({1, 2})
+---
+- error: 'Check constraint failed ''physics'': X > Y'
+...
+s1:drop()
+---
+...
+s2:drop()
+---
+...
test_run:cmd("clear filter")
---
- true
diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua
index a23dcce51..8040fba06 100644
--- a/test/sql/checks.test.lua
+++ b/test/sql/checks.test.lua
@@ -176,4 +176,33 @@ s:insert(s:frommap({X1 = 666, X65 = 666}))
s:insert(s:frommap({X1 = 666, X65 = 666, X63 = 1}))
s:drop()
+--
+-- Test ck constraints LUA integration.
+--
+s1 = box.schema.create_space('test1')
+_ = s1:create_index('pk')
+s1:format({{name='X', type='any'}, {name='Y', type='integer'}})
+s2 = box.schema.create_space('test2')
+_ = s2:create_index('pk')
+s2:format({{name='X', type='any'}, {name='Y', type='integer'}})
+_ = s1:create_check_constraint('physics', 'X < Y')
+_ = s1:create_check_constraint('greater', 'X > 20')
+_ = s2:create_check_constraint('physics', 'X > Y')
+_ = s2:create_check_constraint('greater', 'X > 20')
+s1.ck_constraint.physics ~= nil
+s1.ck_constraint.greater ~= nil
+s2.ck_constraint.physics ~= nil
+s2.ck_constraint.greater ~= nil
+s1:insert({2, 1})
+s1:insert({21, 20})
+s2:insert({1, 2})
+s2:insert({21, 22})
+s2.ck_constraint.greater:drop()
+s2.ck_constraint.physics ~= nil
+s2.ck_constraint.greater == nil
+s1:insert({2, 1})
+s2:insert({1, 2})
+s1:drop()
+s2:drop()
+
test_run:cmd("clear filter")
--
2.21.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 1/4] schema: add new system space for CHECK constraints
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 1/4] schema: add new system space for CHECK constraints Kirill Shcherbatov
@ 2019-05-19 16:01 ` Vladislav Shpilevoy
2019-05-23 10:32 ` Kirill Shcherbatov
0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy @ 2019-05-19 16:01 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches
Hi! Thanks for the patchset!
Great! See 17 comments below!
1. Nikita in the email 13.05 02:04 asked you to create
an issue "sql: add handle to disable constraints". Where
is it? - I can't find.
On 16/05/2019 16:56, Kirill Shcherbatov wrote:
> This patch introduces a new system space to persist check
> constraints. Format of the space:
>
> _ck_constraint (space id = 357)
> [<constraint name> STR, <space id> UINT,
> <is_deferred>BOOL, <code>STR, <language>STR]
2. As I understand, now we want to allow stored functions in Lua, C,
and in future in SQL. But how checks are different? Why there is no
an option like 'language' or something? Lua and C checks are going to
be much faster than SQL ones. Also I see, that Kostja asked you to add
a language at 14.05 23:09 email, but you just ignored it. Why?
>
> CK constraint is local to space, so every pair <CK name, space id>
> is unique (and it is PK in _ck_constraint space).
3. Please, change the space format to [space_id, ck_name, ...] and the
primary index parts to [space_id, ck_name]. Otherwise you can't
lookup all checks of one space having space_id. On the other hand lookup
by name without a space id is useless. This is why _index system space
has such a format and a pk.
>
> After insertion into this space, a new instance describing check
> constraint is created. Check constraint holds AST of expression
> representing it.
> While space features check constraints, it isn't allowed to
> be dropped. The :drop() space method firstly deletes all check
> constraints and then removes entry from _space.
>
> Because space alter, index alter and space truncate operations
> cause space recreation, introduced RebuildCkConstrains object
> that compiles new ck constraint objects, replaces and removes
> existent instances atomically(when some compilation fails,
> nothing is changed). In fact, in scope of this patch we don't
> really need to recreate ck_constraint object in such situations
> (patch space_def pointer in AST like we did it before is enough
> now, but we are going to recompile VDBE that represents ck
> constraint in future that can fail).
>
> The main motivation for these changes is the ability to support
> ADD CHECK CONSTRAINT operation in the future. CK constraints are
> are easier to manage as self-sustained objects: such change is
4. Double 'are'.
> managed with atomic insertion(unlike the current architecture).
>
> Disabled xfer optimization when some space have ck constraints
> because in the following patches this xfer optimisation becomes
> impossible. No reason to rewrite this code.
>
> Needed for #3691
> ---
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 9279426d2..e65c49d14 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -1380,6 +1370,71 @@ UpdateSchemaVersion::alter(struct alter_space *alter)
> ++schema_version;
> }
>
> +/**
> + * As ck_constraint object depends on space_def we must rebuild
> + * all ck constraints on space alter.
> + *
> + * To perform it transactionally, we create a list of a new ck
5. 'a' article is never used with plural.
> + * constraints objects in ::prepare method that is fault-tolerant.
> + * Finally in ::alter or ::rollback methods we only swap thouse
> + * lists securely.
> + */
> +class RebuildCkConstraints: public AlterSpaceOp
> +{
> +public:
> + RebuildCkConstraints(struct alter_space *alter) : AlterSpaceOp(alter),
> + ck_constraint(RLIST_HEAD_INITIALIZER(ck_constraint)) {}
> + struct rlist ck_constraint;
> + virtual void prepare(struct alter_space *alter);
> + virtual void alter(struct alter_space *alter);
> + virtual void rollback(struct alter_space *alter);
> + virtual ~RebuildCkConstraints();
> +};
> @@ -4035,6 +4098,176 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
> }
> }
>
> +/** Create an instance of check constraint definition by tuple. */
> +static struct ck_constraint_def *
> +ck_constraint_def_new_from_tuple(struct tuple *tuple)
> +{
> + uint32_t name_len;
> + const char *name =
> + tuple_field_str_xc(tuple, BOX_CK_CONSTRAINT_FIELD_NAME,
> + &name_len);
> + if (name_len > BOX_NAME_MAX) {
> + tnt_raise(ClientError, ER_CREATE_CK_CONSTRAINT,
> + tt_cstr(name, BOX_INVALID_NAME_MAX),
> + "check constraint name is too long");
> + }
> + identifier_check_xc(name, name_len);
> + const char *language_str =
> + tuple_field_cstr_xc(tuple, BOX_CK_CONSTRAINT_FIELD_LANGUAGE);
> + enum ck_constraint_language language =
> + STR2ENUM(ck_constraint_language, language_str);
> + if (language == ck_constraint_language_MAX) {
> + tnt_raise(ClientError, ER_FUNCTION_LANGUAGE, language_str,
> + tt_cstr(name, name_len));
> + }
6. I do not see a language on the branch. Either the mail, or the
branch, are outdated.
> + uint32_t expr_str_len;
> + const char *expr_str =
> + tuple_field_str_xc(tuple, BOX_CK_CONSTRAINT_FIELD_CODE,
> + &expr_str_len);
> + uint32_t name_offset, expr_str_offset;
> + uint32_t ck_def_sz =
> + ck_constraint_def_sizeof(name_len, expr_str_len, &name_offset,
> + &expr_str_offset);
> + struct ck_constraint_def *ck_def =
> + (struct ck_constraint_def *)malloc(ck_def_sz);
> + if (ck_def == NULL)
> + tnt_raise(OutOfMemory, ck_def_sz, "malloc", "ck_def");
> +
> + ck_def->name = (char *)ck_def + name_offset;
> + ck_def->expr_str = (char *)ck_def + expr_str_offset;
> + ck_def->language = language;
> + memcpy(ck_def->expr_str, expr_str, expr_str_len);
> + ck_def->expr_str[expr_str_len] = '\0';
> + memcpy(ck_def->name, name, name_len);
> + ck_def->name[name_len] = '\0';
> +
> + return ck_def;
> +}
> +
> +/** Trigger invoked on rollback in the _ck_constraint space. */
> +static void
> +on_replace_ck_constraint_rollback(struct trigger *trigger, void *event)
> +{
> + struct txn_stmt *stmt = txn_last_stmt((struct txn *) event);
> + struct ck_constraint *ck = (struct ck_constraint *)trigger->data;
> + struct space *space = NULL;
> + if (ck != NULL)
> + space = space_by_id(ck->space_id);
> + if (stmt->old_tuple != NULL && stmt->new_tuple == NULL) {
> + /* Rollback DELETE check constraint. */
> + assert(ck != NULL);
> + assert(space != NULL);
> + assert(space_ck_constraint_by_name(space,
> + ck->def->name, strlen(ck->def->name)) == NULL);
> + rlist_add_entry(&space->ck_constraint, ck, link);
> + } else if (stmt->new_tuple != NULL && stmt->old_tuple == NULL) {
7. Double whitespace prior to 'else'.
> + /* Rollback INSERT check constraint. */
> + assert(space != NULL);
> + assert(space_ck_constraint_by_name(space,
> + ck->def->name, strlen(ck->def->name)) != NULL);
> + rlist_del_entry(ck, link);
> + ck_constraint_delete(ck);
> + } else {
> + /* Rollback REPLACE check constraint. */
> + assert(space != NULL);
> + const char *name = ck->def->name;
> + struct ck_constraint *new_ck =
> + space_ck_constraint_by_name(space, name, strlen(name));
> + assert(new_ck != NULL);
> + rlist_del_entry(new_ck, link);
> + rlist_add_entry(&space->ck_constraint, ck, link);
> + ck_constraint_delete(new_ck);
> + }
> +}
> +
> +/** A trigger invoked on replace in the _ck_constraint space. */
> +static void
> +on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event)
> +{
> + struct txn *txn = (struct txn *) event;
> + txn_check_singlestatement_xc(txn, "Space _ck_constraint");
> + struct txn_stmt *stmt = txn_current_stmt(txn);
> + struct tuple *old_tuple = stmt->old_tuple;
> + struct tuple *new_tuple = stmt->new_tuple;
> + uint32_t space_id =
> + tuple_field_u32_xc(old_tuple != NULL ? old_tuple : new_tuple,
> + BOX_CK_CONSTRAINT_FIELD_SPACE_ID);
> + struct space *space = space_cache_find_xc(space_id);
> + struct trigger *on_rollback =
> + txn_alter_trigger_new(on_replace_ck_constraint_rollback, NULL);
> + struct trigger *on_commit =
> + txn_alter_trigger_new(on_replace_ck_constraint_commit, NULL);
> +
> + if (new_tuple != NULL) {
> + bool is_deferred =
> + tuple_field_bool_xc(new_tuple,
> + BOX_CK_CONSTRAINT_FIELD_DEFERRED);
> + if (is_deferred) {
> + tnt_raise(ClientError, ER_UNSUPPORTED, "Tarantool",
> + "deferred ck constraints");
> + }
> + /* Create or replace check constraint. */
> + struct ck_constraint_def *ck_def =
> + ck_constraint_def_new_from_tuple(new_tuple);
> + auto ck_guard = make_scoped_guard([=] { free(ck_def); });
> + /*
> + * FIXME: Ck constraint creation on non-empty
> + * space must be implemented as preparatory
> + * step for ALTER SPACE ADD CONSTRAINT feature.
8. We already have ADD CONSTRAINT. It works for FK, UNIQUE, PK. The
problem is that we can't call it on a non-empty space.
> + */
> + struct index *pk = space_index(space, 0);
> + if (pk != NULL && index_size(pk) > 0) {
> + tnt_raise(ClientError, ER_CREATE_CK_CONSTRAINT,
> + ck_def->name,
> + "referencing space must be empty");
> + }
> + struct ck_constraint *new_ck_constraint =
> + ck_constraint_new(ck_def, space->def);
> + if (new_ck_constraint == NULL)
> + diag_raise();
> + ck_guard.is_active = false;
> + const char *name = new_ck_constraint->def->name;
> + struct ck_constraint *old_ck_constraint =
> + space_ck_constraint_by_name(space, name, strlen(name));
> + if (old_ck_constraint != NULL)
> + rlist_del_entry(old_ck_constraint, link);
> + rlist_add_entry(&space->ck_constraint, new_ck_constraint, link);
> + on_commit->data = old_tuple == NULL ? new_ck_constraint :
> + old_ck_constraint;
> + on_rollback->data = on_commit->data;
> + } else {
> + assert(new_tuple == NULL && old_tuple != NULL);
> + /* Drop check constraint. */
> + uint32_t name_len;
> + const char *name =
> + tuple_field_str_xc(old_tuple,
> + BOX_CK_CONSTRAINT_FIELD_NAME,
> + &name_len);
> + struct ck_constraint *old_ck_constraint =
> + space_ck_constraint_by_name(space, name, name_len);
> + assert(old_ck_constraint != NULL);
> + rlist_del_entry(old_ck_constraint, link);
> + on_commit->data = old_ck_constraint;
> + on_rollback->data = old_ck_constraint;
> + }
> +
> + txn_on_rollback(txn, on_rollback);
> + txn_on_commit(txn, on_commit);
> +}
> diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c
> new file mode 100644
> index 000000000..db012837b
> --- /dev/null
> +++ b/src/box/ck_constraint.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above
> + * copyright notice, this list of conditions and the
> + * following disclaimer.
> + *
> + * 2. Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer in the documentation and/or other materials
> + * provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
> + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +#include "box/session.h"
> +#include "ck_constraint.h"
> +#include "errcode.h"
> +#include "sql.h"
> +#include "sql/sqlInt.h"
> +
> +const char *ck_constraint_language_strs[] = {"SQL"};
9. I do not see that on the branch.
> +
> +/**
> + * Resolve space_def references for check constraint via AST
> + * tree traversal.
> + * @param ck_constraint Check constraint object to update.
10. There is no such parameter.
> + * @param space_def Space definition to use.
> + * @retval 0 on success.
> + * @retval -1 on error.
> + */
> +static int
> +ck_constraint_resolve_field_names(struct Expr *expr,
> + struct space_def *space_def)
> +{
> + struct Parse parser;
> + sql_parser_create(&parser, sql_get(), default_flags);
> + parser.parse_only = true;
> + sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL);
> + int rc = parser.is_aborted ? -1 : 0;
> + sql_parser_destroy(&parser);
> + return rc;
> +}
> +
> diff --git a/src/box/ck_constraint.h b/src/box/ck_constraint.h
> new file mode 100644
> index 000000000..9c72d5977
> --- /dev/null
> +++ b/src/box/ck_constraint.h
> @@ -0,0 +1,174 @@
> +/**
> + * Find check constraint object in space by given name and
> + * name_len.
> + * @param space The space to lookup check constraint.
> + * @param name The check constraint name.
> + * @param name_len The length of the name.
> + * @retval not NULL Check constrain if exists, NULL otherwise.
11. constrain -> constraint.
> + */
> +struct ck_constraint *
> +space_ck_constraint_by_name(struct space *space, const char *name,
> + uint32_t name_len);
> +
> +#if defined(__cplusplus)
> +} /* extern "C" { */
> +#endif
> +
> +#endif /* INCLUDES_BOX_CK_CONSTRAINT_H */
> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
> index 89d6e3d52..7f2656eb7 100644
> --- a/src/box/lua/upgrade.lua
> +++ b/src/box/lua/upgrade.lua
> @@ -737,6 +739,46 @@ local function upgrade_to_2_1_3()
> end
> end
>
> +local function upgrade_to_2_2_0()
> + -- In previous Tarantool releases check constraints were
> + -- stored in space opts. Now we use separate space
> + -- _ck_constraint for this purpose. Perform legacy data
> + -- migration.
> + local MAP = setmap({})
> + local _space = box.space[box.schema.SPACE_ID]
> + local _index = box.space[box.schema.INDEX_ID]
> + local _ck_constraint = box.space[box.schema.CK_CONSTRAINT_ID]
> + log.info("create space _ck_constraint")
> + local format = {{name='name', type='string'},
> + {name='space_id', type='unsigned'},
> + {name='is_deferred', type='boolean'},
> + {name='code', type='str'}, {name='language', type='str'}}
> + _space:insert{_ck_constraint.id, ADMIN, '_ck_constraint', 'memtx', 0, MAP, format}
> +
> + log.info("create index primary on _ck_constraint")
> + _index:insert{_ck_constraint.id, 0, 'primary', 'tree',
> + {unique = true}, {{0, 'string'}, {1, 'unsigned'}}}
> +
> + log.info("create secondary index child_id on _ck_constraint")
> + _index:insert{_ck_constraint.id, 1, 'space_id', 'tree',
> + {unique = false}, {{1, 'unsigned'}}}
> + for _, space in _space:pairs() do
> + local flags = space.flags
> + if flags['checks'] ~= nil then
12. Is there any reason why you do not use field access by '.'?
You could use
local _space = box.space._space
local _index = box.space._index
local _ck_constraint = box.space._ck_constraint
...
if flags.checks then
...
flags.checks = nil
instead of
local _space = box.space[box.schema.SPACE_ID]
local _index = box.space[box.schema.INDEX_ID]
local _ck_constraint = box.space[box.schema.CK_CONSTRAINT_ID]
...
if flags['checks'] ~= nil then
...
flags['checks'] = nil
IMO the former looks simpler.
> + for i, check in pairs(flags['checks']) do
> + local expr_str = check.expr
> + local check_name = check.name or
> + "CK_CONSTRAINT_"..i.."_"..space.name
> + _ck_constraint:insert({check_name, space.id, false, expr_str, 'SQL'})
> + end
> + flags['checks'] = nil
> + _space:replace(box.tuple.new({space.id, space.owner, space.name,
> + space.engine, space.field_count,
> + flags, space.format}))
13. Why do you create tuple to replace? Anyway it is not inserted as
is, and :replace() creates another tuple internally.
> + end
> + end
> +end
> +
> diff --git a/src/box/space_def.h b/src/box/space_def.h
> index c6639a8d8..71356e2a6 100644
> --- a/src/box/space_def.h
> +++ b/src/box/space_def.h
> @@ -71,8 +71,6 @@ struct space_opts {
> bool is_view;
> /** SQL statement that produced this space. */
> char *sql;
> - /** SQL Checks expressions list. */
> - struct ExprList *checks;
14. Now you can drop 'struct ExprList;' announcement
from this header.
> };
>
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 6051a2529..f2bbbb9c4 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -638,40 +640,109 @@ primary_key_exit:
> return;
> }
>
> +/**
> + * Prepare a 0-terminated string in the wptr memory buffer that
> + * does not contain a sequence of more than one whatespace
> + * character. Routine enforces ' ' (space) as whitespace
> + * delimiter.> + * The wptr buffer is expected to have str_len + 1 bytes
> + * (this is the expected scenario where no extra whitespace
> + * characters preset in the source string).
> + * @param wptr The destination memory buffer of size
> + * @a str_len + 1.
> + * @param str The source string to be copied.
> + * @param str_len The source string @a str length.
> + */
> +static void
> +trim_space_snprintf(char *wptr, const char *str, uint32_t str_len)
15. You can't just drop all the whitespaces, new lines, and tabs from
an SQL expression, and expect its behaviour unchanged.
tarantool> box.execute("CREATE TABLE test (id INT PRIMARY KEY, a TEXT, CONSTRAINT CK1 CHECK(a != ' '))")
---
- row_count: 1
...
tarantool> box.execute("INSERT INTO test VALUES (1, ' ')")
---
- row_count: 1
...
tarantool> box.space._ck_constraint:select{}
---
- - ['CK1', 512, false, 'a != '' ''']
...
I've forbidden to insert ' ', but you trimmed the string, and
changed the expression logic. Please, either remove
trim_space_snprintf(), or make it smarter and do not touch
space symbols inside quotes. I vote for the second option. As Nikita
fairly noticed, otherwise CHECK bodies are unreadable.
> +{
> + const char *str_end = str + str_len;
> + bool is_prev_chr_space = false;
> + while (str < str_end) {
> + if (isspace((unsigned char)*str)) {
> + if (!is_prev_chr_space)
> + *wptr++ = ' ';
> + is_prev_chr_space = true;
> + str++;
> + continue;
> + }
> + is_prev_chr_space = false;
> + *wptr++ = *str++;
> + }
> + *wptr = '\0';
> +}
> +
> void
> -sql_add_check_constraint(struct Parse *parser)
> +sql_create_check_contraint(struct Parse *parser)
> {
> - struct create_ck_def *ck_def = &parser->create_ck_def;
> + struct create_ck_def *create_ck_def = &parser->create_ck_def;
> + struct ExprSpan *expr_span = create_ck_def->expr;
> + sql_expr_delete(parser->db, expr_span->pExpr, false);
> +
> struct alter_entity_def *alter_def =
> (struct alter_entity_def *) &parser->create_ck_def;
16. Here you can cast 'create_ck_def' without taking it from
&parser->create_ck_def again.
> assert(alter_def->entity_type == ENTITY_TYPE_CK);
> (void) alter_def;
> - struct Expr *expr = ck_def->expr->pExpr;
> struct space *space = parser->create_table_def.new_space;
> - if (space != NULL) {
> - expr->u.zToken =
> - sqlDbStrNDup(parser->db,
> - (char *) ck_def->expr->zStart,
> - (int) (ck_def->expr->zEnd -
> - ck_def->expr->zStart));
> - if (expr->u.zToken == NULL)
> - goto release_expr;
> - space->def->opts.checks =
> - sql_expr_list_append(parser->db,
> - space->def->opts.checks, expr);
> - if (space->def->opts.checks == NULL) {
> - sqlDbFree(parser->db, expr->u.zToken);
> - goto release_expr;
> - }
> - struct create_entity_def *entity_def = &ck_def->base.base;
> - if (entity_def->name.n > 0) {
> - sqlExprListSetName(parser, space->def->opts.checks,
> - &entity_def->name, 1);
> + assert(space != NULL);
> +
> + /* Prepare payload for ck constraint definition. */
> + struct region *region = &parser->region;
> + struct Token *name_token = &create_ck_def->base.base.name;
> + const char *name;
> + if (name_token->n > 0) {
> + name = sql_normalized_name_region_new(region, name_token->z,
> + name_token->n);
> + if (name == NULL) {
> + parser->is_aborted = true;
> + return;
> }
> } else {
> -release_expr:
> - sql_expr_delete(parser->db, expr, false);
> + uint32_t ck_idx = ++parser->create_table_def.check_count;
> + name = tt_sprintf("CK_CONSTRAINT_%d_%s", ck_idx,
> + space->def->name);
> + }
> + size_t name_len = strlen(name);
> +
> + uint32_t expr_str_len = (uint32_t)(create_ck_def->expr->zEnd -
> + create_ck_def->expr->zStart);
17. create_ck_def->expr is expr_span, which you obtained above.
> + const char *expr_str = create_ck_def->expr->zStart;
> +
> + /*
> + * Allocate memory for ck constraint parse structure and
> + * ck constraint definition as a single memory chunk on
> + * region:
> + *
> + * [ck_parse][ck_def[name][expr_str]]
> + * |_____^ |___^ ^
> + * |_________|
> + */
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 4/4] box: user-friendly interface to manage ck constraints
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 4/4] box: user-friendly interface to manage ck constraints Kirill Shcherbatov
@ 2019-05-19 16:02 ` Vladislav Shpilevoy
2019-05-23 10:41 ` Kirill Shcherbatov
0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy @ 2019-05-19 16:02 UTC (permalink / raw)
To: tarantool-patches, Kirill Shcherbatov
Thanks for the patch! See 6 comments below.
On 16/05/2019 16:56, Kirill Shcherbatov wrote:
> Closes #3691
>
> @TarantoolBot document
> Title: check constraint for Lua space
>
> The check constraint is a type of integrity constraint which
> specifies a requirement that must be met by tuple before it
> is inserted into space. The constraint result must be predictable.
> Expression in check constraint must be <boolean value expression>
> I.e. return boolean result.
>
> Now it is possible to create ck constraints only for empty space
> having format. Constraint expression is a string that defines
> relations between top-level tuple fields.
> Take into account that all names are converted to an uppercase
> before resolve(like SQL does), use \" sign for names of fields
> that were created not with SQL.
>
> The check constraints are fired on insertion to the Lua space
> together with Lua space triggers. The execution order of
> ck constraints checks and space triggers follows their creation
> sequence.
>
> Note: this patch changes the CK constraints execution order for
> SQL. Previously check of CK constraints integrity was fired before
> tuple is formed; meanwhile now they are implemented as NoSQL before
> replace triggers, which are fired right before tuple insertion.
> In turn, type casts are performed earlier than msgpack
> serialization. You should be careful with functions that use
> field types in your check constrains (like typeof()).
>
> Consider following situation:
> ```
> box.execute("CREATE TABLE t2(id INT primary key,
> x INTEGER CHECK (x > 1));")
> box.execute("INSERT INTO t2 VALUES(3, 1.1)")
> ```
> the last operation would fail because 1.1 is silently
> cast to integer 1 which is not greater than 1.
1. I do not see that comment on the branch.
>
> To create a new CK constraint for a space, use
> space:create_check_constraint method. All space constraints are
> shown in space.ck_constraint table. To drop ck constraint,
> use :drop method.
>
> Example:
> ```
> s1 = box.schema.create_space('test1')
> pk = s1:create_index('pk')
> ck = s1:create_check_constraint('physics', 'X < Y')
> s1:insert({2, 1}) -- fail
> ck:drop()
> ```
> ---
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index e01f500e6..a30d6aa5b 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua> @@ -1579,10 +1597,17 @@ end
> space_mt.frommap = box.internal.space.frommap
> space_mt.__index = space_mt
>
> +local ck_constraint_mt = {}
> +ck_constraint_mt.drop = function(ck_constraint)
> + check_ck_constraint_arg(ck_constraint, 'drop')
> + box.space._ck_constraint:delete({ck_constraint.name, ck_constraint.space_id})
> +end
> +
> box.schema.index_mt = base_index_mt
> box.schema.memtx_index_mt = memtx_index_mt
> box.schema.vinyl_index_mt = vinyl_index_mt
> box.schema.space_mt = space_mt
> +box.schema.ck_constraint_mt = ck_constraint_mt
2. Please, do not expose ck_constraint_mt.
box.schema.<space/index>_mt are public and documented
metatables to replace certain DML DQL operations with
user's one.
Checks does not provide methods except drop, so it
makes no sense to make their metatable configurable.
It means, that you don't need to wrap checks below.
>
> --
> -- Wrap a global space/index metatable into a space/index local
> @@ -1628,6 +1653,14 @@ function box.schema.space.bless(space)
> end
> end
> end
> + if type(space.ck_constraint) == 'table' and space.enabled then
> + for j, ck_constraint in pairs(space.ck_constraint) do
> + if type(j) == 'string' then
> + setmetatable(ck_constraint,
> + wrap_schema_object_mt('ck_constraint_mt'))
> + end
> + end
> + end
> end
>
> local sequence_mt = {}
> diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
> index ce4287bba..9cec1cbde 100644
> --- a/src/box/lua/space.cc
> +++ b/src/box/lua/space.cc
> @@ -147,6 +148,66 @@ lbox_space_before_replace(struct lua_State *L)
> lbox_push_txn_stmt, lbox_pop_txn_stmt);
> }
>
> +/**
> + * Make ck_constraints available in Lua, via ck_constraint[]
> + * array.
> + * Returns a new table representing a space on top of the Lua
> + * stack.
3. Representing a space? How lbox_ck_constraint can return a space?
As I see, it does not return anything. It just modifies an existing
space table.
> + */
> +static void
> +lbox_ck_constraint(struct lua_State *L, struct space *space, int i)
4. lbox_ck_constraint_what? If a function is not a getter, it should
have a verb saying what the function does.
What is 'i' argument? Index of space table?
> +{
> + lua_getfield(L, i, "ck_constraint");
> + if (lua_isnil(L, -1)) {
> + lua_pop(L, 1);
> + lua_pushstring(L, "ck_constraint");
> + lua_newtable(L);
> + lua_settable(L, i);
> + lua_getfield(L, i, "ck_constraint");
> + } else {
> + lua_pushnil(L);
> + while (lua_next(L, -2) != 0) {
> + size_t name_len;
> + const char *name = lua_tolstring(L, -2, &name_len);
> + /*
> + * Remove ck_constraint only if it was
> + * deleted.
> + */
> + if (space_ck_constraint_by_name(space, name,
> + (uint32_t)name_len) == NULL) {
> + lua_pushlstring(L, name, name_len);
> + lua_pushnil(L);
> + lua_settable(L, -5);
> + }
> + lua_pop(L, 1);
> + }
> + }
> + struct ck_constraint *ck_constraint = NULL;
> + rlist_foreach_entry(ck_constraint, &space->ck_constraint, link) {
> + lua_getfield(L, i, ck_constraint->def->name);
> + if (lua_isnil(L, -1)) {
> + lua_pop(L, 1);
> + lua_pushstring(L, ck_constraint->def->name);
> + lua_newtable(L);
> + lua_settable(L, -3);
> + lua_getfield(L, -1, ck_constraint->def->name);
> + assert(!lua_isnil(L, -1));
> + }
> +
> + lua_pushstring(L, ck_constraint->def->name);
> + lua_setfield(L, -2, "name");
> +
> + lua_pushnumber(L, space->def->id);
> + lua_setfield(L, -2, "space_id");
> +
> + lua_pushstring(L, ck_constraint->def->expr_str);
> + lua_setfield(L, -2, "expr_str");
> +
> + lua_setfield(L, -2, ck_constraint->def->name);
> + }
> + lua_pop(L, 1);
> +}
> +
> /**
> * Make a single space available in Lua,
> * via box.space[] array.
> diff --git a/test/sql/checks.result b/test/sql/checks.result
> index 04f58c90d..d87106c42 100644
> --- a/test/sql/checks.result
> +++ b/test/sql/checks.result
> @@ -513,6 +513,96 @@ s:insert(s:frommap({X1 = 666, X65 = 666, X63 = 1}))
> s:drop()
> ---
> ...
> +--
> +-- Test ck constraints LUA integration.
> +--
> +s1 = box.schema.create_space('test1')
> +---
> +...
> +_ = s1:create_index('pk')
> +---
> +...
> +s1:format({{name='X', type='any'}, {name='Y', type='integer'}})
> +---
> +...
> +s2 = box.schema.create_space('test2')
> +---
> +...
> +_ = s2:create_index('pk')
> +---
> +...
> +s2:format({{name='X', type='any'}, {name='Y', type='integer'}})
> +---
> +...
> +_ = s1:create_check_constraint('physics', 'X < Y')
> +---
> +...
> +_ = s1:create_check_constraint('greater', 'X > 20')
> +---
> +...
> +_ = s2:create_check_constraint('physics', 'X > Y')
> +---
> +...
> +_ = s2:create_check_constraint('greater', 'X > 20')
> +---
> +...
> +s1.ck_constraint.physics ~= nil
> +---
> +- true
5. Why do you avoid serialization? What will happen,
if I will write
tarantool> s1.ck_constraint.physics
?
> +...
> +s1.ck_constraint.greater ~= nil
> +---
> +- true
> +...
> +s2.ck_constraint.physics ~= nil
> +---
> +- true
> +...
> +s2.ck_constraint.greater ~= nil
> +---
> +- true
> +...
> +s1:insert({2, 1})
> +---
> +- error: 'Check constraint failed ''greater'': X > 20'
> +...
> +s1:insert({21, 20})
> +---
> +- error: 'Check constraint failed ''physics'': X < Y'
> +...
> +s2:insert({1, 2})
> +---
> +- error: 'Check constraint failed ''greater'': X > 20'
> +...
> +s2:insert({21, 22})
> +---
> +- error: 'Check constraint failed ''physics'': X > Y'
> +...
> +s2.ck_constraint.greater:drop()
> +---
> +...
> +s2.ck_constraint.physics ~= nil
> +---
> +- true
> +...
> +s2.ck_constraint.greater == nil
> +---
> +- true
> +...
> +s1:insert({2, 1})
> +---
> +- error: 'Check constraint failed ''greater'': X > 20'
> +...
> +s2:insert({1, 2})
> +---
> +- error: 'Check constraint failed ''physics'': X > Y'
> +...
> +s1:drop()
> +---
> +...
> +s2:drop()
6. The test does not check, that after a constraint is
dropped, a tuple can be inserted violating the dropped
constraint.
Also, I do not see a test, that a user can't insert
constraints with the same names.
> +---
> +...
> test_run:cmd("clear filter")
> ---
> - true
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 2/4] box: run check constraint tests on space alter
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 2/4] box: run check constraint tests on space alter Kirill Shcherbatov
@ 2019-05-19 16:02 ` Vladislav Shpilevoy
2019-05-23 10:37 ` Kirill Shcherbatov
0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy @ 2019-05-19 16:02 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches
Thanks for the patch! See 18 comments below.
On 16/05/2019 16:56, Kirill Shcherbatov wrote:
> To perform ck constraints tests before insert or update space operation,
> we use precompiled VDBE machine is associated with each ck constraint,
1. 'we use machine is associated' - ??? Can't parse, please, rephrase.
I see that error quite often in your English. Looks, like you don't know,
that verbs can be adjectives.
Lesson #345223. Verbs as adjectives.
http://www.rhlschool.com/eng4n7.htm
Today we will learn how to say, that an object has a quality
created/added by a verb. In other words, how to turn a verb into
an adjective.
1. Many kind carpenters offered to repair the *broken porch*.
Not "to repair the porch is broken".
2. My father prefers to drink *filtered spring water*.
Not "to drink spring water is filtered".
3. This isn’t chocolate ice cream; it’s *frozen chocolate milk*!
Not "it is chocolate milk is frozen".
4. The *fallen leaves* covered the new driveway.
Not "The leaves is fallen covered the new driveway".
5. She was happy to find the *translated version* of the book.
Not "to find the version is translated of the book".
6. The sleeping dog’s snoring was louder than a *freight train*.
Not "louder than a train is freight".
7. We pushed our way through the *newly driven snow*.
Not "through the snow is driven newly".
8. I’d rather eat at a recently *inspected restaurant*.
Not "at a restaurant is inspected recently".
9. Are you just hoping it will happen or is it *a done deal*?
Not "is it a deal is done".
10. Sadly, as she aged, he became just another *forgotten name*.
Not "became just another name is forgotten".
> that is executed in on_replace trigger. Each ck constraint VDBE code is
> consists of
2. 'is consists' - ??? 'Consist' is a verb, you can't say that.
> 1) prologue code that maps new(or updated) tuple fields via bindings,
> 2) ck constraint code is generated by CK constraint AST.
> In case of ck constraint error the transaction is aborted and ck
3. Why do you abort a transaction because of just one bad DML operation?
A failed operation should not lead to rollback of the entire transaction.
> constraint error is handled as diag message.
> Each ck constraint use own on_replace trigger.
4. It is too expensive, triggers are virtual functions. Please, call
them all from one. Also, it is necessary to be able to turn them
off - you will just remove this single trigger.
>
> Needed for #3691
> ---
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index e65c49d14..746ce62f6 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -1410,14 +1410,29 @@ void
> RebuildCkConstraints::alter(struct alter_space *alter)
> {
> rlist_swap(&alter->new_space->ck_constraint, &ck_constraint);
> + struct ck_constraint *ck;
> + rlist_foreach_entry(ck, &alter->old_space->ck_constraint, link)
> + trigger_clear(&ck->trigger);
> rlist_swap(&ck_constraint, &alter->old_space->ck_constraint);
> + rlist_foreach_entry(ck, &alter->new_space->ck_constraint, link) {
> + /**
> + * Triggers would be swapped later on
> + * alter_space_do.
> + */
5. When 'later'? This function (alter) is called already in
alter_space_do().
> + trigger_add(&alter->old_space->on_replace, &ck->trigger);
> + }
> }
> @@ -1435,6 +1450,35 @@ RebuildCkConstraints::~RebuildCkConstraints()
> }
> }
>
> +/**
> + * Move CK constraints from old space to the new one.
> + * Despite RebuildCkConstraints, this operation doesn't perform
> + * objects rebuild.
6. 'Despite' does not mean the thing you wanted to say here. Please,
rephrase.
> + * This may be used in scenarios where space
> + * format doesn't change i.e. in alter index or space trim
> + * requests.
> + */
> +class MoveCkConstraints: public AlterSpaceOp
> +{
> +public:
> + MoveCkConstraints(struct alter_space *alter) : AlterSpaceOp(alter) {}
> + virtual void alter(struct alter_space *alter);
> + virtual void rollback(struct alter_space *alter);
> +};
> /* }}} */
>
> /**
> @@ -2145,8 +2189,8 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
> * old space.
> */
> alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1);
> + (void) new MoveCkConstraints(alter);
> /* Add an op to update schema_version on commit. */
> - (void) new RebuildCkConstraints(alter);
> (void) new UpdateSchemaVersion(alter);
> alter_space_do(txn, alter);
> scoped_guard.is_active = false;
> @@ -2214,7 +2258,7 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
> (void) new TruncateIndex(alter, old_index->def->iid);
> }
>
> - (void) new RebuildCkConstraints(alter);
> + (void) new MoveCkConstraints(alter);
7. Why is not MoveCkConstraints in the previous patch?
> alter_space_do(txn, alter);
> scoped_guard.is_active = false;
> }
> diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c
> index db012837b..43798be76 100644
> --- a/src/box/ck_constraint.c
> +++ b/src/box/ck_constraint.c
> @@ -29,10 +29,15 @@
> * SUCH DAMAGE.
> */
> #include "box/session.h"
> +#include "bind.h"
> #include "ck_constraint.h"
> #include "errcode.h"
> +#include "schema.h"
> +#include "session.h"
> #include "sql.h"
> #include "sql/sqlInt.h"
> +#include "sql/vdbeInt.h"
> +#include "tuple.h"
>
> const char *ck_constraint_language_strs[] = {"SQL"};
8. Do not see that on the branch.
>
> @@ -57,6 +62,138 @@ ck_constraint_resolve_field_names(struct Expr *expr,
> return rc;
> }
>
> +/**
> + * Create VDBE machine for ck constraint by given definition and
> + * expression AST. The generated instructions consist of
> + * prologue code that maps tuple fields via bindings and ck
> + * constraint code which implements given expression.
> + * In case of ck constraint error during VDBE execution, it is
> + * aborted and error is handled as diag message.
> + * @param ck_constraint_def Check constraint definition to prepare
> + * error description.
> + * @param expr Check constraint expression AST is built for
9. When you say 'AST is built ...', I can't understand, how this
sentence relates to expr. If you wanted to describe 'expr', then
remove 'is'. See comment 1.
> + * given @ck_constraint_def, see for
10. Doxygen does not have '@ck_constraint_def' command. Please, see
the list of commands and do not use not existing. Here you
should use '@a ck_constraint'.
> + * (sql_expr_compile +
> + * ck_constraint_resolve_space_def) implementation.
> + * @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.
> + */
> +static struct sql_stmt *
> +ck_constraint_program_compile(struct ck_constraint_def *ck_constraint_def,
> + struct Expr *expr, struct space_def *space_def)
> +{
> + struct sql *db = sql_get();
> + struct Parse parser;
> + sql_parser_create(&parser, db, default_flags);
> + struct Vdbe *v = sqlGetVdbe(&parser);
> + if (v == NULL) {
> + diag_set(OutOfMemory, sizeof(struct Vdbe), "sqlGetVdbe",
> + "vdbe");
> + return NULL;
> + }
> + /*
> + * Generate a prologue code that introduces variables to
> + * bind tuple fields there before execution.
> + */
> + uint32_t field_count = space_def->field_count;
> + int bind_tuple_reg = sqlGetTempRange(&parser, field_count);
> + for (uint32_t i = 0; i < field_count; i++) {
> + sqlVdbeAddOp2(v, OP_Variable, ++parser.nVar,
> + bind_tuple_reg + i);
> + }
> + /* Generate ck constraint test code. */
> + vdbe_emit_ck_constraint(&parser, expr, ck_constraint_def->name,
> + ck_constraint_def->expr_str, bind_tuple_reg);
> +
> + /* Clean-up and restore user-defined sql context. */
> + bool is_error = parser.is_aborted;
> + sql_finish_coding(&parser);
> + sql_parser_destroy(&parser);
> +
> + if (is_error) {
> + diag_set(ClientError, ER_CREATE_CK_CONSTRAINT,
> + ck_constraint_def->name,
> + box_error_message(box_error_last()));
> + sql_finalize((struct sql_stmt *) v);
> + return NULL;
> + }
> + return (struct sql_stmt *) v;
> +}
> +
> +/**
> + * Run bytecode implementing check constraint on new tuple
> + * before insert or replace in space space_def.
11. 'space space_def' ???
> + * @param ck_constraint Check constraint to run.
> + * @param space_def The space definition of the space this check
> + * constraint is constructed for.
12. There is no parameter named 'space_def'.
> + * @param new_tuple The tuple to be inserted in space.
> + * @retval 0 if check constraint test is passed, -1 otherwise.
13. As I said earlier, multiple times, @retval is not designed to
describe multiple values. @retval means 'return value', one value.
Not 'valueS'. Use one @retval for each single value, or use
@return, which does not require an argument and is used to describe
return values in general.
> + */
> +static int
> +ck_constraint_program_run(struct ck_constraint *ck_constraint,
> + const char *new_tuple)
> +{
> + /*
> + * Prepare parameters for checks->stmt execution:
14. What is 'checks' here? I do not see such a variable here to
dereference it.
> + * Map new tuple fields to Vdbe memory variables in range:
> + * [1, field_count]
> + */
> + struct space *space = space_by_id(ck_constraint->space_id);
> + assert(space != NULL);
> + /*
> + * When last format fields are nullable, they are
> + * 'optional' i.e. they may not be present in the tuple.
> + */
> + uint32_t tuple_field_count = mp_decode_array(&new_tuple);
> + uint32_t field_count =
> + MIN(tuple_field_count, space->def->field_count);
> + for (uint32_t i = 0; i < field_count; i++) {
> + struct sql_bind bind;
> + if (sql_bind_decode(&bind, i + 1, &new_tuple) != 0 ||
> + sql_bind_column(ck_constraint->stmt, &bind, i + 1) != 0) {
> + diag_set(ClientError, ER_CK_CONSTRAINT_FAILED,
> + ck_constraint->def->name,
> + ck_constraint->def->expr_str);
> + return -1;
> + }
> + }
> + /* Checks VDBE can't expire, reset expired flag and go. */
> + struct Vdbe *v = (struct Vdbe *) ck_constraint->stmt;
> + v->expired = 0;
> + while (sql_step(ck_constraint->stmt) == SQL_ROW) {}
15. Why do you iterate? Only DQL has multiple steps, because
it returns tuples. DML, DDL and any other statement can't
return multiple tuples.
> + /*
> + * Get VDBE execution state and reset VM to run it
> + * next time.
> + */
> + return sql_reset(ck_constraint->stmt) != SQL_OK ? -1 : 0;
> +}
> +
> +/**
> + * Ck constraint trigger function. It ss expected to be executed
16. 'ss' -> 'is'.
> + * in space::on_replace trigger.
> + *
> + * It extracts all ck constraint required context from event and
> + * run bytecode implementing check constraint to test a new tuple
> + * before it will be inserted in destination space.
> + */
> +static void
> +ck_constraint_on_replace_trigger(struct trigger *trigger, void *event)
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 7b25d18b3..8409ab343 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -926,35 +901,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);
> - } else {
> - char *name = ck_constraint->def->name;
> - sqlHaltConstraint(parse_context, SQL_CONSTRAINT_CHECK,
17. SQL_CONSTRAINT_CHECK is unused now. P5_ConstraintCheck too.
> - 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 0753705ec..d353d7906 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -3894,6 +3905,21 @@ 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, const char *expr_str,
18. expr_str is not described.
> + int new_tuple_reg);
> /**
> * This routine generates code to finish the INSERT or UPDATE
> * operation that was started by a prior call to
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 3/4] box: introduce column_mask for ck constraint
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 3/4] box: introduce column_mask for ck constraint Kirill Shcherbatov
@ 2019-05-19 16:02 ` Vladislav Shpilevoy
2019-05-23 10:38 ` Kirill Shcherbatov
2019-05-26 12:03 ` Vladislav Shpilevoy
0 siblings, 2 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2019-05-19 16:02 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches
Thanks for the patch! See 3 comments below.
On 16/05/2019 16:56, Kirill Shcherbatov wrote:
> This patch introduces a smart bitmask of fields are mentioned in
1. Remove 'are'. 'mentioned' here works as adjective. The same in
other places. Please, find them by yourself.
> ck constraint. This allows to do not bind useless fields and
> do not parse more fields that is required.
>
> Needed for #3691
> ---
> src/box/ck_constraint.c | 66 ++++++++++++++++++++++++++++++++++------
> src/box/ck_constraint.h | 6 ++++
> src/box/sql.h | 5 ++-
> src/box/sql/build.c | 2 +-
> src/box/sql/resolve.c | 19 +++++++++---
> src/box/sql/sqlInt.h | 6 ++++
> test/sql/checks.result | 59 +++++++++++++++++++++++++++++++++++
> test/sql/checks.test.lua | 20 ++++++++++++
> 8 files changed, 167 insertions(+), 16 deletions(-)
>
> diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c
> index 43798be76..4756f7970 100644
> --- a/src/box/ck_constraint.c
> +++ b/src/box/ck_constraint.c
> @@ -46,17 +46,21 @@ const char *ck_constraint_language_strs[] = {"SQL"};
> * tree traversal.
> * @param ck_constraint Check constraint object to update.
> * @param space_def Space definition to use.
> + * @param column_mask[out] The "smart" column mask of fields are
> + * referenced by AST.
> * @retval 0 on success.
> * @retval -1 on error.
> */
> static int
> ck_constraint_resolve_field_names(struct Expr *expr,
> - struct space_def *space_def)
> + struct space_def *space_def,
> + uint64_t *column_mask)
> {
> struct Parse parser;
> sql_parser_create(&parser, sql_get(), default_flags);
> parser.parse_only = true;
> - sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL);
> + sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL,
> + column_mask);
2. Next to last argument 'expr_list' is always NULL. Please, drop it.
> int rc = parser.is_aborted ? -1 : 0;
> sql_parser_destroy(&parser);
> return rc;
> @@ -98,10 +105,24 @@ ck_constraint_program_compile(struct ck_constraint_def *ck_constraint_def,
> * bind tuple fields there before execution.
> */
> uint32_t field_count = space_def->field_count;
> + /*
> + * Use column mask to prepare binding only for
> + * referenced fields.
> + */
> int bind_tuple_reg = sqlGetTempRange(&parser, field_count);
> - for (uint32_t i = 0; i < field_count; i++) {
> + struct bit_iterator it;
> + bit_iterator_init(&it, &column_mask, sizeof(uint64_t), true);
> + size_t used_fieldno = bit_iterator_next(&it);
> + for (; used_fieldno < SIZE_MAX; used_fieldno = bit_iterator_next(&it)) {
> sqlVdbeAddOp2(v, OP_Variable, ++parser.nVar,
> - bind_tuple_reg + i);
> + bind_tuple_reg + used_fieldno);
> + }
> + if (column_mask_fieldno_is_set(column_mask, 63)) {
3. Please, do not hardcode that value. Add a one-line function to
column_mask.h for that. On other places too.
> + used_fieldno = 64;
> + for (; used_fieldno < (size_t)field_count; used_fieldno++) {
> + sqlVdbeAddOp2(v, OP_Variable, ++parser.nVar,
> + bind_tuple_reg + used_fieldno);
> + }
> }
> /* Generate ck constraint test code. */
> vdbe_emit_ck_constraint(&parser, expr, ck_constraint_def->name,
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 1/4] schema: add new system space for CHECK constraints
2019-05-19 16:01 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-05-23 10:32 ` Kirill Shcherbatov
2019-05-26 12:03 ` Vladislav Shpilevoy
0 siblings, 1 reply; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-23 10:32 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
On 19.05.2019 19:01, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patchset!
>
> Great! See 17 comments below!
>
> 1. Nikita in the email 13.05 02:04 asked you to create
> an issue "sql: add handle to disable constraints". Where
> is it? - I can't find.
https://github.com/tarantool/tarantool/issues/4244
>> _ck_constraint (space id = 357)
>> [<constraint name> STR, <space id> UINT,
>> <is_deferred>BOOL, <code>STR, <language>STR]
>
> 2. As I understand, now we want to allow stored functions in Lua, C,
> and in future in SQL. But how checks are different? Why there is no
> an option like 'language' or something? Lua and C checks are going to
> be much faster than SQL ones. Also I see, that Kostja asked you to add
> a language at 14.05 23:09 email, but you just ignored it. Why?
In fact, you've reviewed an outdated branch. As you see, there was
such code in the letter and moreover it is in may new letter for you.
>
> 3. Please, change the space format to [space_id, ck_name, ...] and the
> primary index parts to [space_id, ck_name]. Otherwise you can't
> lookup all checks of one space having space_id. On the other hand lookup
> by name without a space id is useless. This is why _index system space
> has such a format and a pk.
Done.
>> are easier to manage as self-sustained objects: such change is
>
> 4. Double 'are'.
Done.
>> + * To perform it transactionally, we create a list of a new ck
>
> 5. 'a' article is never used with plural.
It is not plural here. A list ...
>
> 6. I do not see a language on the branch. Either the mail, or the
> branch, are outdated.
See 2.
>> + } else if (stmt->new_tuple != NULL && stmt->old_tuple == NULL) {
>
> 7. Double whitespace prior to 'else'.
Fixed.
>> + /*
>> + * FIXME: Ck constraint creation on non-empty
>> + * space must be implemented as preparatory
>> + * step for ALTER SPACE ADD CONSTRAINT feature.
>
> 8. We already have ADD CONSTRAINT. It works for FK, UNIQUE, PK. The
> problem is that we can't call it on a non-empty space.
https://github.com/tarantool/tarantool/issues/4243
>> +const char *ck_constraint_language_strs[] = {"SQL"};
>
> 9. I do not see that on the branch.
See 2.
>> + * Resolve space_def references for check constraint via AST
>> + * tree traversal.
>> + * @param ck_constraint Check constraint object to update.
>
> 10. There is no such parameter.
Fixed.
>> + * Find check constraint object in space by given name and
>> + * name_len.
>> + * @param space The space to lookup check constraint.
>> + * @param name The check constraint name.
>> + * @param name_len The length of the name.
>> + * @retval not NULL Check constrain if exists, NULL otherwise.
>
> 11. constrain -> constraint.
fixed.
> 12. Is there any reason why you do not use field access by '.'?
> You could use
>
> local _space = box.space._space
> local _index = box.space._index
> local _ck_constraint = box.space._ck_constraint
> ...
> if flags.checks then
> ...
> flags.checks = nil
>
> instead of
>
> local _space = box.space[box.schema.SPACE_ID]
> local _index = box.space[box.schema.INDEX_ID]
> local _ck_constraint = box.space[box.schema.CK_CONSTRAINT_ID]
> ...
> if flags['checks'] ~= nil then
> ...
> flags['checks'] = nil
>
> IMO the former looks simpler.
No reason. I did it looking on some legacy code. Done.
>> + _space:replace(box.tuple.new({space.id, space.owner, space.name,
>> + space.engine, space.field_count,
>> + flags, space.format}))
>
> 13. Why do you create tuple to replace? Anyway it is not inserted as
> is, and :replace() creates another tuple internally.
Fixed,
>> - /** SQL Checks expressions list. */
>> - struct ExprList *checks;
>
> 14. Now you can drop 'struct ExprList;' announcement
> from this header.
Ok.
>> +static void
>> +trim_space_snprintf(char *wptr, const char *str, uint32_t str_len)
>
> 15. You can't just drop all the whitespaces, new lines, and tabs from
> an SQL expression, and expect its behaviour unchanged.
>
> tarantool> box.execute("CREATE TABLE test (id INT PRIMARY KEY, a TEXT, CONSTRAINT CK1 CHECK(a != ' '))")
> ---
> - row_count: 1
> ...
>
> tarantool> box.execute("INSERT INTO test VALUES (1, ' ')")
> ---
> - row_count: 1
> ...
>
> tarantool> box.space._ck_constraint:select{}
> ---
> - - ['CK1', 512, false, 'a != '' ''']
> ...
>
> I've forbidden to insert ' ', but you trimmed the string, and
> changed the expression logic. Please, either remove
> trim_space_snprintf(), or make it smarter and do not touch
> space symbols inside quotes. I vote for the second option. As Nikita
> fairly noticed, otherwise CHECK bodies are unreadable.
You are right. Fixed. New "smart" routine is on branch.
>> struct alter_entity_def *alter_def =
>> (struct alter_entity_def *) &parser->create_ck_def;
>
> 16. Here you can cast 'create_ck_def' without taking it from
> &parser->create_ck_def again.
Ok.
>> + uint32_t expr_str_len = (uint32_t)(create_ck_def->expr->zEnd -
>> + create_ck_def->expr->zStart);
>
> 17. create_ck_def->expr is expr_span, which you obtained above.
>
>> + const char *expr_str = create_ck_def->expr->zStart;
Ok.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 2/4] box: run check constraint tests on space alter
2019-05-19 16:02 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-05-23 10:37 ` Kirill Shcherbatov
0 siblings, 0 replies; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-23 10:37 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
On 19.05.2019 19:02, Vladislav Shpilevoy wrote:
> Thanks for the patch! See 18 comments below.
>
> On 16/05/2019 16:56, Kirill Shcherbatov wrote:
>> To perform ck constraints tests before insert or update space operation,
>> we use precompiled VDBE machine is associated with each ck constraint,
>
> 1. 'we use machine is associated' - ??? Can't parse, please, rephrase.
> I see that error quite often in your English. Looks, like you don't know,
> that verbs can be adjectives.
Thank you.
>> that is executed in on_replace trigger. Each ck constraint VDBE code is
>> consists of
>
> 2. 'is consists' - ??? 'Consist' is a verb, you can't say that.
Fixed.
>
>> 1) prologue code that maps new(or updated) tuple fields via bindings,
>> 2) ck constraint code is generated by CK constraint AST.
>> In case of ck constraint error the transaction is aborted and ck
>
> 3. Why do you abort a transaction because of just one bad DML operation?
> A failed operation should not lead to rollback of the entire transaction.
It is not so actually. In fact, I just abort the current replace/insertion.
> 4. It is too expensive, triggers are virtual functions. Please, call
> them all from one. Also, it is necessary to be able to turn them
> off - you will just remove this single trigger.
Done. Now I use the only trigger.
> 5. When 'later'? This function (alter) is called already in
> alter_space_do().
Outdated.
>> +/**
>> + * Move CK constraints from old space to the new one.
>> + * Despite RebuildCkConstraints, this operation doesn't perform
>> + * objects rebuild.
>
> 6. 'Despite' does not mean the thing you wanted to say here. Please,
> rephrase.
Unlike actually.
>> - (void) new RebuildCkConstraints(alter);
>> + (void) new MoveCkConstraints(alter);
>
> 7. Why is not MoveCkConstraints in the previous patch?
Discussed verbally. New ck constraint object doesn't store
Expr AST so we may do not rebuild an object. Previously
there were space_def pointers that become outdated.
>> const char *ck_constraint_language_strs[] = {"SQL"};
>
> 8. Do not see that on the branch.
See the gopher? And he is =)
Maybe I didn't updated the branch, but I am not sure about it
because this change is old enough.
>> + * @param expr Check constraint expression AST is built for
>
> 9. When you say 'AST is built ...', I can't understand, how this
> sentence relates to expr. If you wanted to describe 'expr', then
> remove 'is'. See comment 1.
Ok...
>> + * given @ck_constraint_def, see for
>
> 10. Doxygen does not have '@ck_constraint_def' command. Please, see
> the list of commands and do not use not existing. Here you
> should use '@a ck_constraint'.
FIxed, thank you.
>> +/**
>> + * Run bytecode implementing check constraint on new tuple
>> + * before insert or replace in space space_def.
>
> 11. 'space space_def' ???
Outdated.
>
>> + * @param ck_constraint Check constraint to run.
>> + * @param space_def The space definition of the space this check
>> + * constraint is constructed for.
>
> 12. There is no parameter named 'space_def'.
Fixed.
>
>> + * @param new_tuple The tuple to be inserted in space.
>> + * @retval 0 if check constraint test is passed, -1 otherwise.
>
> 13. As I said earlier, multiple times, @retval is not designed to
> describe multiple values. @retval means 'return value', one value.
> Not 'valueS'. Use one @retval for each single value, or use
> @return, which does not require an argument and is used to describe
> return values in general.
Ok...
>> + /*
>> + * Prepare parameters for checks->stmt execution:
>
> 14. What is 'checks' here? I do not see such a variable here to
> dereference it.
Outdated.
>> + while (sql_step(ck_constraint->stmt) == SQL_ROW) {}
>
> 15. Why do you iterate? Only DQL has multiple steps, because
> it returns tuples. DML, DDL and any other statement can't
> return multiple tuples.
You are right. It is redundant.
>> +/**
>> + * Ck constraint trigger function. It ss expected to be executed
>
> 16. 'ss' -> 'is'.
Fixed.
> 17. SQL_CONSTRAINT_CHECK is unused now. P5_ConstraintCheck too.
Done.
>> +void
>> +vdbe_emit_ck_constraint(struct Parse *parser, struct Expr *expr,
>> + const char *name, const char *expr_str,
>
> 18. expr_str is not described.
Fixed.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 3/4] box: introduce column_mask for ck constraint
2019-05-19 16:02 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-05-23 10:38 ` Kirill Shcherbatov
2019-05-26 12:03 ` Vladislav Shpilevoy
1 sibling, 0 replies; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-23 10:38 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
No such path on the branch now.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 4/4] box: user-friendly interface to manage ck constraints
2019-05-19 16:02 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-05-23 10:41 ` Kirill Shcherbatov
2019-05-26 12:04 ` Vladislav Shpilevoy
0 siblings, 1 reply; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-23 10:41 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
Hi! Thank you for the review.
>> the last operation would fail because 1.1 is silently
>> cast to integer 1 which is not greater than 1.
>
> 1. I do not see that comment on the branch.
...
>> +box.schema.ck_constraint_mt = ck_constraint_mt
>
> 2. Please, do not expose ck_constraint_mt.
> box.schema.<space/index>_mt are public and documented
> metatables to replace certain DML DQL operations with
> user's one.
>
> Checks does not provide methods except drop, so it
> makes no sense to make their metatable configurable.
>
> It means, that you don't need to wrap checks below.
Ok. Done.
>> +/**
>> + * Make ck_constraints available in Lua, via ck_constraint[]
>> + * array.
>> + * Returns a new table representing a space on top of the Lua
>> + * stack.
>
> 3. Representing a space? How lbox_ck_constraint can return a space?
> As I see, it does not return anything. It just modifies an existing
> space table.
>
>> + */
>> +static void
>> +lbox_ck_constraint(struct lua_State *L, struct space *space, int i)
Updated comment.
>
> 4. lbox_ck_constraint_what? If a function is not a getter, it should
> have a verb saying what the function does.
>
> What is 'i' argument? Index of space table?
This code is all similar to the code that is near it...
> 5. Why do you avoid serialization? What will happen,
> if I will write
>
> tarantool> s1.ck_constraint.physics
I don't like space_id to be show. It may differ.
>
> 6. The test does not check, that after a constraint is
> dropped, a tuple can be inserted violating the dropped
> constraint.
It is not so, actually I test exactly this case.
>
> Also, I do not see a test, that a user can't insert
> constraints with the same names.
Appended.
Tnx!
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 1/4] schema: add new system space for CHECK constraints
2019-05-23 10:32 ` Kirill Shcherbatov
@ 2019-05-26 12:03 ` Vladislav Shpilevoy
2019-05-31 13:45 ` Kirill Shcherbatov
0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy @ 2019-05-26 12:03 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches
>>> + * To perform it transactionally, we create a list of a new ck
>>
>> 5. 'a' article is never used with plural.
> It is not plural here. A list ...
Somewhy you trimmed the context, but here it is again:
"we create a list of a new ck constraints objects"
You wrote "a new ck constraints objects". Firstly, "a objects"
is incorrect. Secondly, if you want to describe an object as
purposed to something, you do not need plural. It should be
"we create a list of new ck constraint objects"
"constraints" -> "constraint", it is like an adjective here.
"a objects" -> "objects". You can't use 'a' with plural.
>>> + /*
>>> + * FIXME: Ck constraint creation on non-empty
>>> + * space must be implemented as preparatory
>>> + * step for ALTER SPACE ADD CONSTRAINT feature.
>>
>> 8. We already have ADD CONSTRAINT. It works for FK, UNIQUE, PK. The
>> problem is that we can't call it on a non-empty space.
> https://github.com/tarantool/tarantool/issues/4243
My comment was rather about incorrect comment - "ADD CONSTRAINT"
is already implemented, but here you state, that it is not so. The
comment is still incorrect.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 3/4] box: introduce column_mask for ck constraint
2019-05-19 16:02 ` [tarantool-patches] " Vladislav Shpilevoy
2019-05-23 10:38 ` Kirill Shcherbatov
@ 2019-05-26 12:03 ` Vladislav Shpilevoy
2019-05-31 13:45 ` Kirill Shcherbatov
1 sibling, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy @ 2019-05-26 12:03 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches
>> diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c
>> index 43798be76..4756f7970 100644
>> --- a/src/box/ck_constraint.c
>> +++ b/src/box/ck_constraint.c
>> @@ -46,17 +46,21 @@ const char *ck_constraint_language_strs[] = {"SQL"};
>> * tree traversal.
>> * @param ck_constraint Check constraint object to update.
>> * @param space_def Space definition to use.
>> + * @param column_mask[out] The "smart" column mask of fields are
>> + * referenced by AST.
>> * @retval 0 on success.
>> * @retval -1 on error.
>> */
>> static int
>> ck_constraint_resolve_field_names(struct Expr *expr,
>> - struct space_def *space_def)
>> + struct space_def *space_def,
>> + uint64_t *column_mask)
>> {
>> struct Parse parser;
>> sql_parser_create(&parser, sql_get(), default_flags);
>> parser.parse_only = true;
>> - sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL);
>> + sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL,
>> + column_mask);
>
> 2. Next to last argument 'expr_list' is always NULL. Please, drop it.
I still do not see that argument dropped.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 4/4] box: user-friendly interface to manage ck constraints
2019-05-23 10:41 ` Kirill Shcherbatov
@ 2019-05-26 12:04 ` Vladislav Shpilevoy
2019-05-31 13:45 ` Kirill Shcherbatov
0 siblings, 1 reply; 20+ messages in thread
From: Vladislav Shpilevoy @ 2019-05-26 12:04 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches
>>
>> 4. lbox_ck_constraint_what? If a function is not a getter, it should
>> have a verb saying what the function does.
>>
>> What is 'i' argument? Index of space table?
> This code is all similar to the code that is near it...
It does not mean, that it is good. Besides, you did not
answer the question, and still did nothing in the new
version of the patch about that place.
>
>> 5. Why do you avoid serialization? What will happen,
>> if I will write
>>
>> tarantool> s1.ck_constraint.physics
> I don't like space_id to be show. It may differ.
You can hide it with a test-run filter, or remember it
into a copy table, nullify space_id, and print the rest.
Anyway, you need to check serialization.
>
>>
>> 6. The test does not check, that after a constraint is
>> dropped, a tuple can be inserted violating the dropped
>> constraint.
> It is not so, actually I test exactly this case.
Where? I opened the test file, and there is only one
place, where you drop a constraint, but after it all
insertions fail. I do not see a successful insertion.
This is all the code after a single ck constraint drop:
s2.ck_constraint.greater:drop()
---
...
s2.ck_constraint.physics ~= nil
---
- true
...
s2.ck_constraint.greater == nil
---
- true
...
s1:insert({2, 1})
---
- error: 'Check constraint failed ''greater'': X > 20'
...
s2:insert({1, 2})
---
- error: 'Check constraint failed ''physics'': X > Y'
...
s1:drop()
---
...
s2:drop()
All the insertions fail.
By the way, what happens, if a constraint object is assigned
to a local variable, then its space is dropped. What if I
then call :drop() on the constraint object?
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 1/4] schema: add new system space for CHECK constraints
2019-05-26 12:03 ` Vladislav Shpilevoy
@ 2019-05-31 13:45 ` Kirill Shcherbatov
0 siblings, 0 replies; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-31 13:45 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
> Somewhy you trimmed the context, but here it is again:
>
> "we create a list of a new ck constraints objects"
>
> You wrote "a new ck constraints objects". Firstly, "a objects"
> is incorrect. Secondly, if you want to describe an object as
> purposed to something, you do not need plural. It should be
>
> "we create a list of new ck constraint objects"
>
> "constraints" -> "constraint", it is like an adjective here.
> "a objects" -> "objects". You can't use 'a' with plural.
Fixed.
>>> 8. We already have ADD CONSTRAINT. It works for FK, UNIQUE, PK. The
>>> problem is that we can't call it on a non-empty space.
>> https://github.com/tarantool/tarantool/issues/4243
>
> My comment was rather about incorrect comment - "ADD CONSTRAINT"
> is already implemented, but here you state, that it is not so. The
> comment is still incorrect.
Ok, now it is
/*
* FIXME: Ck constraint creation on non-empty
* space is not implemented yet.
*/
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 3/4] box: introduce column_mask for ck constraint
2019-05-26 12:03 ` Vladislav Shpilevoy
@ 2019-05-31 13:45 ` Kirill Shcherbatov
0 siblings, 0 replies; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-31 13:45 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
>>> + sql_resolve_self_reference(&parser, space_def, NC_IsCheck, expr, NULL,
>>> + column_mask);
>>
>> 2. Next to last argument 'expr_list' is always NULL. Please, drop it.
>
> I still do not see that argument dropped.
Done.
void
sql_resolve_self_reference(struct Parse *parser, struct space_def *def,
int type, struct Expr *expr)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 4/4] box: user-friendly interface to manage ck constraints
2019-05-26 12:04 ` Vladislav Shpilevoy
@ 2019-05-31 13:45 ` Kirill Shcherbatov
2019-06-03 21:15 ` Vladislav Shpilevoy
0 siblings, 1 reply; 20+ messages in thread
From: Kirill Shcherbatov @ 2019-05-31 13:45 UTC (permalink / raw)
To: tarantool-patches, Vladislav Shpilevoy
>>> 4. lbox_ck_constraint_what? If a function is not a getter, it should
>>> have a verb saying what the function does.
>>>
>>> What is 'i' argument? Index of space table?
>> This code is all similar to the code that is near it...
>
> It does not mean, that it is good. Besides, you did not
> answer the question, and still did nothing in the new
> version of the patch about that place.
/**
* Make ck_constraints available in Lua, via ck_constraint[]
* array for space table by given index i.
* Updata a ck_constraint table in the parent space table object
* on the Lua stack.
*/
static void
lbox_push_ck_constraint(struct lua_State *L, struct space *space, int i)
>
>>
>>> 5. Why do you avoid serialization? What will happen,
>>> if I will write
>>>
>>> tarantool> s1.ck_constraint.physics
>> I don't like space_id to be show. It may differ.
>
> You can hide it with a test-run filter, or remember it
> into a copy table, nullify space_id, and print the rest.
> Anyway, you need to check serialization.
Nice idea. I've solved this problem using filter:
test_run:cmd("push filter 'space_id: [0-9]+' to 'space_id: <ID>'")
>>> 6. The test does not check, that after a constraint is
>>> dropped, a tuple can be inserted violating the dropped
>>> constraint.
>> It is not so, actually I test exactly this case.
>
> Where? I opened the test file, and there is only one
> place, where you drop a constraint, but after it all
> insertions fail. I do not see a successful insertion.
> This is all the code after a single ck constraint drop:
....
> All the insertions fail.
s2.ck_constraint.greater:drop()
---
...
s2:insert({1, 2})
---
- error: 'Check constraint failed ''physics'': X > Y'
...
s2:insert({2, 1})
---
- [2, 1]
...
>
> By the way, what happens, if a constraint object is assigned
> to a local variable, then its space is dropped. What if I
> then call :drop() on the constraint object?
s2:drop()
---
...
physics_ck
---
- []
...
physics_ck:drop()
---
- error: '[string "return physics_ck:drop() "]:1: attempt to call method ''drop''
(a nil value)'
...
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v4 4/4] box: user-friendly interface to manage ck constraints
2019-05-31 13:45 ` Kirill Shcherbatov
@ 2019-06-03 21:15 ` Vladislav Shpilevoy
0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2019-06-03 21:15 UTC (permalink / raw)
To: tarantool-patches, Kirill Shcherbatov
>>
>> By the way, what happens, if a constraint object is assigned
>> to a local variable, then its space is dropped. What if I
>> then call :drop() on the constraint object?
> s2:drop()
> ---
> ...
> physics_ck
> ---
> - []
> ...
> physics_ck:drop()
> ---
> - error: '[string "return physics_ck:drop() "]:1: attempt to call method ''drop''
> (a nil value)'
> ...
WTF? Drop method should exist. You saved in physics_ck not a constraint,
but a list of them. Fix that, this test is incorrect.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-06-03 21:15 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-16 13:56 [tarantool-patches] [PATCH v4 0/4] box: run checks on insertions in LUA spaces Kirill Shcherbatov
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 1/4] schema: add new system space for CHECK constraints Kirill Shcherbatov
2019-05-19 16:01 ` [tarantool-patches] " Vladislav Shpilevoy
2019-05-23 10:32 ` Kirill Shcherbatov
2019-05-26 12:03 ` Vladislav Shpilevoy
2019-05-31 13:45 ` Kirill Shcherbatov
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 2/4] box: run check constraint tests on space alter Kirill Shcherbatov
2019-05-19 16:02 ` [tarantool-patches] " Vladislav Shpilevoy
2019-05-23 10:37 ` Kirill Shcherbatov
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 3/4] box: introduce column_mask for ck constraint Kirill Shcherbatov
2019-05-19 16:02 ` [tarantool-patches] " Vladislav Shpilevoy
2019-05-23 10:38 ` Kirill Shcherbatov
2019-05-26 12:03 ` Vladislav Shpilevoy
2019-05-31 13:45 ` Kirill Shcherbatov
2019-05-16 13:56 ` [tarantool-patches] [PATCH v4 4/4] box: user-friendly interface to manage ck constraints Kirill Shcherbatov
2019-05-19 16:02 ` [tarantool-patches] " Vladislav Shpilevoy
2019-05-23 10:41 ` Kirill Shcherbatov
2019-05-26 12:04 ` Vladislav Shpilevoy
2019-05-31 13:45 ` Kirill Shcherbatov
2019-06-03 21:15 ` Vladislav Shpilevoy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox