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 341A525A5E for ; Thu, 7 Jun 2018 11:01:27 -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 PMUTjR8GFM5x for ; Thu, 7 Jun 2018 11:01:27 -0400 (EDT) Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (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 DCB5225A2D for ; Thu, 7 Jun 2018 11:01:26 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH 1/3] sql: fetch primary index for affinity only References: <5bc1cc0b-529b-62b7-6ed3-36f657db1cd2@tarantool.org> <20180607120324.h2laztbxfnhnpe2s@tarantool.org> From: Vladislav Shpilevoy Message-ID: <5b1597db-8cba-1c71-2337-5d570fa3bc3a@tarantool.org> Date: Thu, 7 Jun 2018 18:01:24 +0300 MIME-Version: 1.0 In-Reply-To: <20180607120324.h2laztbxfnhnpe2s@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed 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, Kirill Yukhin Hello. Thanks for the fixes! See one minor comment below. And I do not see the new commit on the branch. Branch still has the old one + 2 my review fixes commits. > -- > Regards, Kirill Yukhin > > commit 6ef9616b71c6e253336c012442b90c4d2ebb2c55 > Author: Kirill Yukhin > Date: Wed May 23 15:31:37 2018 +0300 > > sql: fetch primary index for affinity only > > This small patch removes usages of primary index throughout > code sql_table_delete_from, limiting use to fetching of > affinities only. We cannot use space_def here, since primary > index might contain calculated columns. > > Part of #3235 > > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c > index ddad54b..a4a8da6 100644 > --- a/src/box/sql/delete.c > +++ b/src/box/sql/delete.c > @@ -325,8 +338,14 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, > (void *)space, P4_SPACEPTR); > sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor, > table->tnum, space_ptr_reg); > - sql_vdbe_set_p4_key_def(parse, pk); > - VdbeComment((v, "%s", pk->zName)); > + struct key_def *def = key_def_dup(pk_def); > + if (def == NULL) { > + sqlite3OomFault(parse->db); > + goto delete_from_cleanup; > + } > + sqlite3VdbeAppendP4(v, def, P4_KEYDEF); Why could not you at first dup key_def, and then just use AddOp4? Instead of AddOp3 + dup + AppendP4. > + > + VdbeComment((v, "%s", space->index[0]->def->name)); > > if (one_pass == ONEPASS_MULTI) > sqlite3VdbeJumpHere(v, iAddrOnce);