From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4C16546970E for ; Tue, 24 Dec 2019 03:47:42 +0300 (MSK) Date: Tue, 24 Dec 2019 03:47:41 +0300 From: Nikita Pettik Message-ID: <20191224004741.GC41539@tarantool.org> References: <659c12232934c069ae8d71ed1e2bc8115d36f6e0.1574846892.git.korablev@tarantool.org> <20191218151745.GC28206@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20191218151745.GC28206@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 5/6] sql: extend result set with autoincrement List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Ostanevich Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org On 18 Dec 18:17, Sergey Ostanevich wrote: > Hi! > > Thanks for the patch, the latest version in branch LGTM. > Just one nit regarding readability below - not relevant to the patch > itself. > > regards, > Sergos > > > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > > index b772bcead..f40178194 100644 > > --- a/src/box/sql/select.c > > +++ b/src/box/sql/select.c > > @@ -1841,6 +1841,12 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList, > > if (p->op == TK_COLUMN) > > vdbe_set_metadata_col_nullability(v, i, > > is_nullable); > > + if (pTabList->a[j].space->sequence != NULL) { > > The 'j' variable is a liveout from a loop that searches for the table > the expression in progress refers to. Renaming it to something like > 'current_table' would save some time during review and further > development. I try to avoid mixing functional changes and pure refactoring - it simplifies review and makes git history way cleaner. In this case, I consider renaming j to something more meaningful to be a pure refactoring. > > + int afno = > > + pTabList->a[j].space->sequence_fieldno; > > + if (afno == iCol) > > + vdbe_set_metadata_col_autoincrement(v, i); > > + } > > } else { > > const char *z = pEList->a[i].zSpan; > > if (z == NULL)