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 44BA026CA5 for ; Tue, 20 Aug 2019 15:41:53 -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 h0DxFjRl5eAh for ; Tue, 20 Aug 2019 15:41:53 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 0272A26AC8 for ; Tue, 20 Aug 2019 15:41:52 -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: <326482ED-8190-4B72-B2B8-F20763A86F7E@tarantool.org> Date: Tue, 20 Aug 2019 22:41:50 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20190802125252.54621-1-roman.habibov@tarantool.org> <6B834CCD-6A77-42D0-8AAD-B1BD2B8F51A6@tarantool.org> <8BD992A5-0A74-4E60-A239-5FDE85783467@tarantool.org> <326482ED-8190-4B72-B2B8-F20763A86F7E@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 16 Aug 2019, at 22:09, Roman Khabibov = wrote: >> On Aug 14, 2019, at 01:10, n.pettik wrote: >>> 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. >>=20 >> Still, one can a bit modify query and get old error: >>=20 >> 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 > +test:do_catchsql_test( > + "view-24.4", > + [[ > + 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); > + ]], { > + -- > + 1,"Failed to create view 'V' with " > + -- > + }) >=20 >> 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: >>=20 >> 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; >>=20 >> This is quite unpleasant. >>=20 >>> commit e311d35bd64422bf6468b28a28057d9045817eca >>> Author: Roman Khabibov >>> Date: Mon Jul 29 18:00:34 2019 +0300 >>>=20 >>> sql: disallow in select >>=20 >> -> sql: disallow WITH clause in CREATE VIEW statement >>=20 >>> We don't support queries with the syntax: >>> "CREATE VIEW v AS WITH ... SELECT =E2=80=A6" >>=20 >> Strictly speaking, we do support some of such queries. >> See example at the end of letter. >>=20 >>> Throw the syntax error in this case. >>=20 >> 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(). > If I understand you correctly, we should check presence in any = select, > which can meet after . update_view_references() is called = in > on_drop_view_rollback(), that don't relate to our case. So, I decided = to > check it before update_view_references(), as you can see in diff. You slightly misunderstood me. I proposed to allow such queries gracefully handling WITH statement and avoiding references counter incrementation for CTEs.