[tarantool-patches] Re: [PATCH v7 7/7] sql: remove Checks to server

Kirill Shcherbatov kshcherbatov at tarantool.org
Mon May 28 17:59:45 MSK 2018


> 1. I do not see a test on it
> 3. One test is not enough. You did not test my example with 'exp' above, you did not test 'expr'
> type mismatch, unexpected options in a check (expr + name + some unexpected one) etc.
--- a/test/sql/checks.test.lua
+++ b/test/sql/checks.test.lua
@@ -18,4 +18,27 @@ t = {513, 1, 'test', 'memtx', 0, opts, format}
 s = box.space._space:insert(t)
 box.space._space:delete(513)
 
+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)
+
+-- 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)
+
+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)
+
+-- 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)


>> We don't replicate checks in SQL. At the end of sqlite3EndTable checks are released and the only checks collection present on the server side.
> 2. Are you sure? I added assert(def->checks == NULL) in deleteTable() and it fails:

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index bd99f20..8acd01e 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -408,10 +408,11 @@ deleteTable(sqlite3 * db, Table * pTable)
        sqlite3SelectDelete(db, pTable->pSelect);
        assert(pTable->def != NULL);
        /* Do not delete pTable->def allocated on region. */
-       if (!pTable->def->opts.temporary)
+       if (!pTable->def->opts.temporary) {
                space_def_delete(pTable->def);
-       else
-               assert(pTable->def->opts.checks == NULL);
+       } else {
+               sql_expr_list_delete(db, pTable->def->opts.checks);
+       }
        sqlite3DbFree(db, pTable);





More information about the Tarantool-patches mailing list