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 0C06B25140 for ; Tue, 13 Aug 2019 18:10:19 -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 RALgCfFTGcjG for ; Tue, 13 Aug 2019 18:10:18 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 B707A21B87 for ; Tue, 13 Aug 2019 18:10:18 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: [tarantool-patches] Re: [PATCH] sql: add check for absence in From: "n.pettik" In-Reply-To: <6B834CCD-6A77-42D0-8AAD-B1BD2B8F51A6@tarantool.org> Date: Wed, 14 Aug 2019 01:10:15 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <8BD992A5-0A74-4E60-A239-5FDE85783467@tarantool.org> References: <20190802125252.54621-1-roman.habibov@tarantool.org> <6B834CCD-6A77-42D0-8AAD-B1BD2B8F51A6@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: Roman Khabibov > On 13 Aug 2019, at 15:42, Roman Khabibov = wrote: >> On Aug 9, 2019, at 18:15, n.pettik wrote: >>> On 2 Aug 2019, at 15:52, Roman Khabibov = wrote: >>>=20 >>> Check that hasn't after : "CREATE VIEW v >>> AS WITH ... SELECT ...". Throw error, if it has. >>=20 >> What is the reason for that? I run example from ticket: >>=20 >> tarantool> \set language sql >> tarantool> \set delimiter ; >>=20 >> tarantool> CREATE TABLE ts (s1 INT PRIMARY KEY); >> tarantool> INSERT INTO ts VALUES (1); >> tarantool> WITH RECURSIVE w AS ( >>> SELECT s1 FROM ts >>> UNION ALL >>> SELECT s1+1 FROM w WHERE s1 < 4) >>> SELECT * FROM w; >> tarantool> CREATE VIEW v AS WITH RECURSIVE w AS ( >>> SELECT s1 FROM ts >>> UNION ALL >>> SELECT s1+1 FROM w WHERE s1 < 4) >>> SELECT * FROM w; >> - null >> - Space 'W' does not exist >> ... >>=20 >> So, it fails at the stage of view creation. Please, ask Peter >> to provide valid example. Otherwise there=E2=80=99s no problem and >> issue can be closed. >>=20 > = https://github.com/tarantool/tarantool/issues/4149#issuecomment-520390204 >=20 >> This looks like a crutch. Please, either find out if it is >> possible to fix this at parser level (changing grammar), >> or simply close issue. > Yes, now it=E2=80=99s one-line fix, but error isn't so detailed. > But now it's a syntax error, as was required in the ticket. Still, one can a bit modify query and get old error: tarantool> CREATE VIEW v AS SELECT * FROM ( WITH RECURSIVE w AS ( SELECT = s1 FROM ts UNION ALL SELECT s1+1 FROM w WHERE s1 < 4) SELECT * fROM w); --- - error: Space 'W' does not exist =E2=80=A6 What is more, if user created view with WITH clause (see example at the end of letter) and updated tnt to your branch, user would be forced to use force_recovery option: 2019-08-14 00:30:42.835 [27409] main/102/interactive box.cc:329 E> error = applying row: {type: 'INSERT', replica_id: 1, lsn: 19, space_id: 280, = index_id: 0, tuple: [514, 1, "V", "memtx", 1, {"sql": "CREATE VIEW v AS = WITH v1(id) as (select 1) SELECT * FROM ts", "view": true}, [{"name": = "S1", "type": "integer", "is_nullable": true, "nullable_action": = "none"}]]} 2019-08-14 00:30:42.835 [27409] main/102/interactive tokenize.c:571 E> = ER_SQL_EXECUTE: Failed to execute SQL statement: CREATE VIEW v AS WITH = v1(id) as (select 1) SELECT * FROM ts 2019-08-14 00:30:42.835 [27409] main/102/interactive F> can't initialize = storage: Failed to execute SQL statement: CREATE VIEW v AS WITH v1(id) = as (select 1) SELECT * FROM ts; This is quite unpleasant. > commit e311d35bd64422bf6468b28a28057d9045817eca > Author: Roman Khabibov > Date: Mon Jul 29 18:00:34 2019 +0300 >=20 > sql: disallow in select -> sql: disallow WITH clause in CREATE VIEW statement > We don't support queries with the syntax: > "CREATE VIEW v AS WITH ... SELECT =E2=80=A6" Strictly speaking, we do support some of such queries. See example at the end of letter. > Throw the syntax error in this case. Instead of banning CTEs in view=E2=80=99s body, we can fix their = creation. The problem is that during view creation all referenced spaces are fetched by name from SELECT and their reference counters are incremented to avoid dangling references. It occurs in update_view_references(). Obviously, CTE tables are not held in space cache, ergo error =E2=80=9Cspace doesn=E2=80=99t exist=E2=80=9D = is raised. So, what we can do is add graceful check to update_view_references(). > Closes #4149 >=20 > diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y > index 4c9d95f9c..d9ce239ca 100644 > --- a/src/box/sql/parse.y > +++ b/src/box/sql/parse.y > @@ -399,7 +399,7 @@ ifexists(A) ::=3D . {A =3D 0;} > ///////////////////// The CREATE VIEW statement = ///////////////////////////// > // > cmd ::=3D CREATE(X) VIEW ifnotexists(E) nm(Y) eidlist_opt(C) > - AS select(S). { > + AS selectnowith(S). { > if (!pParse->parse_only) { > create_view_def_init(&pParse->create_view_def, &Y, &X, C, S, E); > pParse->initiateTTrans =3D true; > diff --git a/test/sql-tap/view.test.lua b/test/sql-tap/view.test.lua > index 101f4c3e7..cdba27e04 100755 > --- a/test/sql-tap/view.test.lua > +++ b/test/sql-tap/view.test.lua > @@ -1,6 +1,6 @@ > #!/usr/bin/env tarantool > test =3D require("sqltester") > -test:plan(73) > +test:plan(76) >=20 > --!./tcltestrunner.lua > -- 2002 February 26 > @@ -1233,4 +1233,39 @@ test:do_catchsql_test( > -- > }) >=20 > +-- gh-4149: Check error message for > +-- "CREATE VIEW v AS WITH ... SELECT ...". > +test:do_execsql_test( > + "view-24.1", > + [[ > + CREATE TABLE ts (s1 INT PRIMARY KEY); > + INSERT INTO ts VALUES (1); > + ]], { > + -- > + -- > + }) > + > +test:do_catchsql_test( > + "view-24.2", > + [[ > + CREATE VIEW v AS WITH RECURSIVE w AS ( > + SELECT s1 FROM ts > + UNION ALL > + SELECT s1+1 FROM w WHERE s1 < 4) > + SELECT * FROM w; > + ]], { > + -- > + 1,"Keyword 'WITH' is reserved. Please use double quotes if = 'WITH' is an identifier." > + -- > + }) Look at WITH rule in parse.y: with(A) ::=3D . {A =3D 0;} with(A) ::=3D WITH wqlist(W). { A =3D W; } with(A) ::=3D WITH RECURSIVE wqlist(W). { A =3D W; } So, basically you should also check cases like: CREATE VIEW v AS WITH v1(id) as (select 1) SELECT * FROM ts; (It works on master branch, but this query is absolutely meaningless)