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 BAA7E2DD81 for ; Sun, 26 May 2019 12:07:16 -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 uw3I2HpGdH39 for ; Sun, 26 May 2019 12:07:16 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 030B127A9D for ; Sun, 26 May 2019 12:07:15 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.8\)) Subject: [tarantool-patches] Re: [PATCH v1 06/21] sql: remove SQL_NOTFOUND error/status code From: "n.pettik" In-Reply-To: <20190525202603.GA2340@atlas> Date: Sun, 26 May 2019 19:07:12 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <76BC8B9F-80C7-450A-A303-049052F37159@tarantool.org> References: <4E2824A4-6F08-4539-9954-EF9783217D54@tarantool.org> <20190525202603.GA2340@atlas> 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: Konstantin Osipov > On 25 May 2019, at 23:26, Konstantin Osipov = wrote: >=20 > * n.pettik [19/05/25 21:58]: >> Taking into consideration assert above, it could >> be replaced with assert(pBuilder->nRecValid =3D=3D nEq - 1); >> Btw, this function doesn=E2=80=99t seem to be called at all: >> unreachable() assert doesn=E2=80=99t fire. I can assume that it is >> connected with stat tables. >>=20 >=20 > NIkita, could you please be more specific, is it an accept or a > reject? I=E2=80=99d say it is rather accept. This fix touches QP and now I can=E2=80=99t validate it since statistics functionality is disabled. > Can review comments be done in their own patches?=20 Is it common practice for our work-flow? I mean I see that as a rule fixe are done and squashed with main patches before they are pushed. > This is a big patch stack and I would like us to stop shuffling it > around. If it does improve things, let it get in please and do the > follow up work separately. Or at least let some of the good > patches in. I don=E2=80=99t mind if those patches which I didn=E2=80=99t comment out will be pushed separately. There=E2=80=99re not so many concerns from my side, so I guess whole patch-set will be ready soon.=