From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 37A8622B1C for ; Mon, 28 May 2018 10:59:47 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id E80frf-_ImAk for ; Mon, 28 May 2018 10:59:47 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id E601C21CB6 for ; Mon, 28 May 2018 10:59:46 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v7 7/7] sql: remove Checks to server References: <2baee418a3799c91c5a8b92ac62e59de3456da0d.1527084287.git.kshcherbatov@tarantool.org> <4221becc-9d9c-852a-ccd1-6ed3618cde9f@tarantool.org> <6b860615-cd91-414a-3948-32600b69e05f@tarantool.org> From: Kirill Shcherbatov Message-ID: Date: Mon, 28 May 2018 17:59:45 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: "v.shpilevoy@tarantool.org" > 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);