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 1296025CB0 for ; Mon, 22 Apr 2019 03:49:24 -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 1fivShGPROjK for ; Mon, 22 Apr 2019 03:49:23 -0400 (EDT) Received: from smtp15.mail.ru (smtp15.mail.ru [94.100.176.133]) (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 5829525ADF for ; Mon, 22 Apr 2019 03:49:22 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/3] sql: remove mayAbort field from struct Parse References:

From: Imeev Mergen Message-ID: <2ca61ada-3bc3-5e42-15c2-f24f090c107f@tarantool.org> Date: Mon, 22 Apr 2019 10:49:20 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/alternative; boundary="------------6D8BFD6A503849364AA4C668" Content-Language: en-US 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: "n.pettik" , tarantool-patches@freelists.org This is a multi-part message in MIME format. --------------6D8BFD6A503849364AA4C668 Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Hi! Thank you for review! On 4/15/19 5:06 PM, n.pettik wrote: > >> On 12 Apr 2019, at 15:34, imeevma@tarantool.org wrote: >> >> Currently, the mayAbort field is used only in one place in debug >> mode and is not used in non-debug mode. This patch removes this >> field. > Could you be more specific when pointing out the reason of removal? > What was the feature you are removing and why it can be removed? > Argument like ‘it is used only in debug mode’ doesn’t seem to be > convincing enough. I'll add that this allows us to remove SQL errcode SQL_CONSTRAINT. > >> Part of #4074 > Code involved in this patch doesn’t throw any errors, > so why it is a part of diag replacement? > > I guess this code clean-up can be OK, but we must > be sure that this functionality can’t be applied to our > SQL implementation. > In fact, this code works with the error code SQL_CONSTRAINT, and if we want to remove this error code, we must think of a way to replace it with some other similar checks. I think this will make the code less understandable. --------------6D8BFD6A503849364AA4C668 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit

Hi! Thank you for review!

On 4/15/19 5:06 PM, n.pettik wrote:

On 12 Apr 2019, at 15:34, imeevma@tarantool.org wrote:

Currently, the mayAbort field is used only in one place in debug
mode and is not used in non-debug mode. This patch removes this
field.

Could you be more specific when pointing out the reason of removal?
What was the feature you are removing and why it can be removed?
Argument like ‘it is used only in debug mode’ doesn’t seem to be
convincing enough.

I'll add that this allows us to remove SQL errcode SQL_CONSTRAINT.

Part of #4074

Code involved in this patch doesn’t throw any errors,
so why it is a part of diag replacement?

I guess this code clean-up can be OK, but we must
be sure that this functionality can’t be applied to our
SQL implementation.

In fact, this code works with the error code SQL_CONSTRAINT, and
if we want to remove this error code, we must think of a way to
replace it with some other similar checks. I think this will make
the code less understandable.

--------------6D8BFD6A503849364AA4C668--