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 71AF620A77 for ; Thu, 12 Sep 2019 10:15:11 -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 qmgjnuxuSjPk for ; Thu, 12 Sep 2019 10:15:11 -0400 (EDT) Received: from smtp43.i.mail.ru (smtp43.i.mail.ru [94.100.177.103]) (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 3043320A63 for ; Thu, 12 Sep 2019 10:15:11 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 1/3] box: an ability to disable CK constraints References: <9ff97ab5cf810e52988267f150c330826d2e4910.1568275504.git.kshcherbatov@tarantool.org> <20190912140044.GA97194@tarantool.org> From: Kirill Shcherbatov Message-ID: <7a9c02d6-9ae9-6c88-cc57-ea379b2967f5@tarantool.org> Date: Thu, 12 Sep 2019 17:15:08 +0300 MIME-Version: 1.0 In-Reply-To: <20190912140044.GA97194@tarantool.org> 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: Nikita Pettik , Tarantool MailList >> + struct memtx_engine *memtx = >> + (struct memtx_engine *)engine_by_name("memtx"); >> + assert(memtx != NULL); > > Wait a second, how it is supposed to work with vinyl? All system spaces use memtx engine. Thus looking for memtx engine state is an acceptable way to determine, whether Tarantool loaded. > Nit: method is calles "enable", but it can also disable trigger. > I'd rename it to "_set_status"/"set_state" etc I don't like both your variants. "enable" has been proposed by Kostya. >> + if (space_foreach(space_run_ck_constraints, (void *)true) != 0) >> + unreachable(); > > What does this change do? Why do you need param if it is always true? In the first patch version I used to call this function space_run_ck_constraints with false and true in two places to enable/disable all check constraint. In my opinion hardcoding true in smth like space_enable_ck_constraints is not really good idea. > Is space_is_system() check vital? It may be omitted. Initially I assumed that system spaces cold not have ck constraints but actually it is not so now, they could.