From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (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 95CAC46970E for ; Wed, 18 Dec 2019 03:29:44 +0300 (MSK) References: <79354099f03b9efe3a300ef4fd2c7d934876ae7b.1576071711.git.korablev@tarantool.org> From: Vladislav Shpilevoy Message-ID: <33e3b5df-45c3-83ff-3d0b-cbc1af6a1bae@tarantool.org> Date: Wed, 18 Dec 2019 01:29:41 +0100 MIME-Version: 1.0 In-Reply-To: <79354099f03b9efe3a300ef4fd2c7d934876ae7b.1576071711.git.korablev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 5/6] sql: extend result set with autoincrement List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik , tarantool-patches@dev.tarantool.org Thanks for the patch! See 3 comments below. On 11/12/2019 14:44, Nikita Pettik wrote: > If result set contains column which features attached sequence > (AUTOINCREMENT in terms of SQL) then meta-information will contain > corresponding field ('is_autoicrement' : boolean) in response. > > Part of #4407 > --- > diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c > index 644b373c9..88ef4ff78 100644 > --- a/src/box/lua/net_box.c > +++ b/src/box/lua/net_box.c > @@ -651,11 +651,15 @@ decode_metadata_optional(struct lua_State *L, const char **data, > const char *coll = mp_decode_str(data, &len); > lua_pushlstring(L, coll, len); > lua_setfield(L, -2, "collation"); > - } else { > - assert(key == IPROTO_FIELD_IS_NULLABLE); > + } else if (key == IPROTO_FIELD_IS_NULLABLE) { > bool is_nullable = mp_decode_bool(data); > lua_pushboolean(L, is_nullable); > lua_setfield(L, -2, "is_nullable"); > + } else { > + assert(key == IPROTO_FIELD_IS_AUTOINCREMENT); > + bool autoincrement = mp_decode_bool(data); 1. autoincrement -> is_autoincrement. > + lua_pushboolean(L, autoincrement); > + lua_setfield(L, -2, "is_autoincrement"); > } > } > } > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > index 73ce95eba..d92da4d8e 100644 > --- a/src/box/sql/select.c > +++ b/src/box/sql/select.c > @@ -1845,6 +1845,12 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList, > space_def->fields[iCol].is_nullable; > vdbe_metadata_set_col_nullability(v, i, > is_nullable); > + if (pTabList->a[j].space->sequence != NULL) { > + int afno = > + pTabList->a[j].space->sequence_fieldno; > + if (afno == iCol) > + vdbe_metadata_set_col_autoincrement(v, i); > + } > } > } else { > const char *z = NULL; 2. Consider this refactoring to make it more accurate: ============================================================================ @@ -1822,7 +1822,8 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList, break; } assert(j < pTabList->nSrc); - struct space_def *space_def = pTabList->a[j].space->def; + struct space *space = pTabList->a[j].space; + struct space_def *space_def = space->def; assert(iCol >= 0 && iCol < (int)space_def->field_count); zCol = space_def->fields[iCol].name; const char *name = NULL; @@ -1845,12 +1846,9 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList, space_def->fields[iCol].is_nullable; vdbe_metadata_set_col_nullability(v, i, is_nullable); - if (pTabList->a[j].space->sequence != NULL) { - int afno = - pTabList->a[j].space->sequence_fieldno; - if (afno == iCol) - vdbe_metadata_set_col_autoincrement(v, i); - } + if (space->sequence != NULL && + space->sequence_fieldno == iCol) + vdbe_metadata_set_col_autoincrement(v, i); } } else { const char *z = NULL; ============================================================================ > diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c > index 2de41a70a..e3672097c 100644 > --- a/src/box/sql/vdbeaux.c > +++ b/src/box/sql/vdbeaux.c > @@ -1911,6 +1911,13 @@ vdbe_metadata_set_col_nullability(struct Vdbe *p, int idx, int nullable) > p->metadata[idx].nullable = nullable; > } > > +void > +vdbe_metadata_set_col_autoincrement(struct Vdbe *p, int idx) > +{ > + assert(idx < p->nResColumn); > + p->metadata[idx].is_actoincrement = 1; 3. Why 1 instead of true?