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 6957F257F1 for ; Wed, 30 May 2018 06:42:36 -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 tB9EEiytFMIX for ; Wed, 30 May 2018 06:42:36 -0400 (EDT) Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (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 B55D42540D for ; Wed, 30 May 2018 06:42:35 -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 v7 7/7] sql: remove Checks to server From: "n.pettik" In-Reply-To: <36da1e00-8787-91e0-c267-78c87836e4e3@tarantool.org> Date: Wed, 30 May 2018 13:42:31 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <1E697F67-DF86-4DFD-BD2C-2112834D5302@tarantool.org> References: <2baee418a3799c91c5a8b92ac62e59de3456da0d.1527084287.git.kshcherbatov@tarantool.org> <90226C84-EE19-4DC3-BD50-147C7EAF5DB8@tarantool.org> <36da1e00-8787-91e0-c267-78c87836e4e3@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: Kirill Shcherbatov > On 30 May 2018, at 11:32, Kirill Shcherbatov = wrote: >=20 >> You can specify that =E2=80=98opt=E2=80=99 is output param by [out] = tag: >> @param[out] opt ... > - * @param opt pointer to store parsing result. > + * @param[out] opt pointer to store parsing result. >=20 >> Mb it would be better to rename this struct according to our = codestyle? > ExprList is a SQL structure not only for checks, so it's not a good = idea to make a project-wide renaming.=20 But sooner or later we will have to do it. Anyway, as you wish. >>=20 >>> +++ b/src/box/sql.c >>> @@ -1500,22 +1500,47 @@ int tarantoolSqlite3MakeTableFormat(Table = *pTable, void *buf) >>> * >>> * Ex: {"temporary": true, "sql": "CREATE TABLE student (name, = grade)"} >>> */ >>> -int tarantoolSqlite3MakeTableOpts(Table *pTable, const char *zSql, = void *buf) >>> +int >>> +tarantoolSqlite3MakeTableOpts(Table *pTable, const char *zSql, char = *buf) >>> { >>> const struct Enc *enc =3D get_enc(buf); >>> - char *base =3D buf, *p; >>> + bool is_view =3D pTable !=3D NULL && = pTable->def->opts.is_view; >>> + bool has_checks =3D pTable !=3D NULL && = pTable->def->opts.checks !=3D NULL; >>=20 >> In fact, it is incorrect since opts.checks may also contain aliases = for VIEW >> columns: CREATE VIEW V1(a, b, c) AS SELECT * FROM t1; >> In this case, checks contain NULL exprs with names =E2=80=98A=E2=80=99,= =E2=80=98B=E2=80=99 and =E2=80=98C=E2=80=99. >> So you should add another one condition: && is_view. >=20 >> The same is here: you add checks for VIEW which are really aliases. > The VIEW checks are also represented as ExprList with name pointers = but lacks Expr pointer. > So, this check is valid.=20 I extremely dislike this original SQLite approach, since it is so = confusing. I suggest to rework it somehow. I have dived into this problem. Now aliases are stored as checks only for one reason: to tell whether SELECT of view is resolved or not. If not, aliases are extracted from checks list and put into regular = fields array. Thus, if we store aliases as fields right after view creation, we can=E2=80=99t point out whether we have resolved names for this view, or it is just view with aliases. But since point of this patch slightly different, lets leave it as it = is. >=20 >> Why do we still need to create table wrappers? >> Is it possible to change signatures and use straight space_def = instead of fake tables? > sql_resolve_self_reference initialize SrcList field pTab;=20 > Unfortunately sql_resolve_self_reference is not the only way of usage = SrcList and at least Table field pIndex is required in other scenarios. Well, l see now. Seems it can be fixed after introducing index_def in = struct Index.