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 80CC327021 for ; Fri, 27 Jul 2018 09:33:59 -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 je82H5Xeg0HC for ; Fri, 27 Jul 2018 09:33:59 -0400 (EDT) Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (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 3C33B26FCB for ; Fri, 27 Jul 2018 09:33:59 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH] sql: remove preupdate hook From: "n.pettik" In-Reply-To: <20180727131942.6ubuclhzzrmp3yqy@tarantool.org> Date: Fri, 27 Jul 2018 16:33:50 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <694D42E1-950E-4593-931D-C92602AB317D@tarantool.org> References: <7D1DA51D-5B70-43C0-BA5D-1E3E497CF327@tarantool.org> <20180727131942.6ubuclhzzrmp3yqy@tarantool.org> 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: Kirill Yukhin >> I have grepped and found few mentions about preupdate hook >> which you didn=E2=80=99t delete. >>=20 >> Firstly, it seems that you can remove struct PreUpdate. >> Secondly, one comment in update.c (line 565): >>=20 >> * That (regNew=3D=3DregnewPk+1) is true is also important for the >> * pre-update hook. If the caller invokes preupdate_new(), the = returned >> * value is copied from memory cell (regNewPk+1+iCol), where iCol >> * is the column index supplied by the user. >>=20 >> Remove it as well pls. > Fixed. Updated patch below. Thx, now LGTM.=