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 11C252B6ED for ; Tue, 27 Mar 2018 07:04:50 -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 sFz4iQdj2pds for ; Tue, 27 Mar 2018 07:04:50 -0400 (EDT) Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41]) (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 C5A9D2B6EA for ; Tue, 27 Mar 2018 07:04:49 -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 v2 1/2] Introduce 'view' space option From: "n.pettik" In-Reply-To: <20180327060256.GA4581@atlas> Date: Tue, 27 Mar 2018 14:04:46 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <715A7546-5404-4D7B-9853-8FBB5E615C41@tarantool.org> References: <20180327060256.GA4581@atlas> 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: Konstantin Osipov Cc: tarantool-patches@freelists.org > On 27 Mar 2018, at 09:02, Konstantin Osipov = wrote: >=20 > * Nikita Pettik [18/03/27 02:06]: >> + if (opts.view && opts.sql =3D=3D NULL) { >> + tnt_raise(ClientError, ER_VIEW_MISSING_SQL); >> + } >=20 > We usually do not add {} braces around one-line if bodies. Fixed: - if (opts.view && opts.sql =3D=3D NULL) { + if (opts.is_view && opts.sql =3D=3D NULL) tnt_raise(ClientError, ER_VIEW_MISSING_SQL); - } >=20 >> struct space_def *def =3D >> space_def_new_xc(id, uid, exact_field_count, name, = name_len, >> engine_name, engine_name_len, &opts, = fields, >> @@ -1590,6 +1593,10 @@ on_replace_dd_index(struct trigger * /* = trigger */, void *event) >> uint32_t iid =3D tuple_field_u32_xc(old_tuple ? old_tuple : = new_tuple, >> BOX_INDEX_FIELD_ID); >> struct space *old_space =3D space_cache_find_xc(id); >> + if (old_space->def->opts.view) { >> + tnt_raise(ClientError, ER_ALTER_SPACE, = space_name(old_space), >> + "can not alter indexes on view"); >> + } >=20 > can not add index on a view. >=20 > You know for sure it's add, since it can't be drop or modify, > so let's the error message more specific. >=20 > You can also introduce a separate error code for the purpose, > ER_ALTER_VIEW. Fixed: - if (old_space->def->opts.view) { + if (old_space->def->opts.is_view) { tnt_raise(ClientError, ER_ALTER_SPACE, = space_name(old_space), - "can not alter indexes on view"); + "can not add index on a view"); >=20 >> const struct space_opts space_opts_default =3D { >> /* .temporary =3D */ false, >> + /* .view =3D */ false, >> /* .sql =3D */ NULL, >> }; >=20 > is_view. Usually we add is_ or has_ prefix to boolean state > variables. But field =E2=80=99temporary=E2=80=99 in the same struct isn=E2=80=99t = called =E2=80=98is_temporary=E2=80=99, so I just didn=E2=80=99t want to outline this field. Anyway, renamed to = =E2=80=98is_view=E2=80=99. >=20 >> + if (new_def->opts.view !=3D old_def->opts.view) { >> + diag_set(ClientError, ER_ALTER_SPACE, old_def->name, >> + "can not transform space to view and visa = versa"); >> + return -1; >> + } >=20 > can't convert a space to a view or vice vice versa. >=20 > In future, please check all error messages you add with Elena. >=20 Fixed: - if (new_def->opts.view !=3D old_def->opts.view) { + if (new_def->opts.is_view !=3D old_def->opts.is_view) { diag_set(ClientError, ER_ALTER_SPACE, old_def->name, - "can not transform space to view and visa = versa"); + "can not convert a space to a view and vice = versa");