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 613D3210DE for ; Tue, 11 Dec 2018 13:29:33 -0500 (EST) 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 r5aU1IF_78KT for ; Tue, 11 Dec 2018 13:29:33 -0500 (EST) 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 turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 20654210C0 for ; Tue, 11 Dec 2018 13:29:33 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH 5/6] sql: don't add string of 'CREATE INDEX ...' to index opts From: "n.pettik" In-Reply-To: Date: Tue, 11 Dec 2018 21:29:31 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <11243F86-41D8-4008-906E-65DA7A9E0119@tarantool.org> References: <988bd992af26db529d76e259d2498a7e04f958b1.1544387419.git.korablev@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: Vladislav Shpilevoy > On 10 Dec 2018, at 17:18, Vladislav Shpilevoy = wrote: >=20 > Thanks for the patch! See 2 comments and review fixes below. >=20 > On 10/12/2018 00:30, Nikita Pettik wrote: >> Part of #2647 >> --- >> src/box/sql.c | 22 +++------------------- >> src/box/sql/build.c | 35 +++-------------------------------- >> 2 files changed, 6 insertions(+), 51 deletions(-) >> diff --git a/src/box/sql.c b/src/box/sql.c >> index e79265823..45c539ec7 100644 >> --- a/src/box/sql.c >> +++ b/src/box/sql.c >> @@ -1214,7 +1196,9 @@ sql_encode_index_opts(struct region *region, = const struct index_opts *opts, >> bool is_error =3D false; >> mpstream_init(&stream, region, region_reserve_cb, = region_alloc_cb, >> set_encode_error, &is_error); >> - mpstream_encode_index_opts(&stream, opts); >> + mpstream_encode_map(&stream, 1); >> + mpstream_encode_str(&stream, "unique"); >> + mpstream_encode_bool(&stream, opts->is_unique);; >> mpstream_flush(&stream); >=20 > 1. We do not need mpstream for this tiny piece of code. I > refactored it to simple region_alloc. Ok, I don=E2=80=99t mind. >> if (is_error) { >> diag_set(OutOfMemory, stream.pos - stream.buf, = "mpstream_flush", >> diff --git a/src/box/sql/build.c b/src/box/sql/build.c >> index fd1666c6d..ab2a56420 100644 >> --- a/src/box/sql/build.c >> +++ b/src/box/sql/build.c >> @@ -2546,6 +2527,7 @@ sql_create_index(struct Parse *parse, struct = Token *token, >> sqlite3VdbeAddOp1(vdbe, OP_Close, cursor); >> vdbe_emit_create_index(parse, def, index->def, >> def->id, index_id); >> + sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE); >=20 > 2. Are you sure that you should increment change counter here? > As I know, it should be incremented once on a DDL, but this > increment is under 'if (tbl_name !=3D NULL)' which means that it > is not a separate index, but part of CREATE TABLE, it is not? > CREATE TABLE increments change counter on insertion into _space > only. tbl_name !=3D NULL means that currently we are processing CREATE INDEX statement, not CREATE TABLE. So, its ok. >=20 >> sqlite3VdbeAddOp0(vdbe, OP_Expire); >> } >> =20 >=20 > My review fixes here and on the branch: Applied.