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 29070241A7 for ; Mon, 21 May 2018 14:27:04 -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 XtBnnJYx1_eR for ; Mon, 21 May 2018 14:27:04 -0400 (EDT) Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [94.100.177.92]) (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 1F595241A4 for ; Mon, 21 May 2018 14:27:02 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 1/1] sql: COLLATE after LIMIT throws an error References: <13ccf5856ab21d72c71a3fc5f65739871d824b40.1526913411.git.imeevma@tarantool.org> <1526915797.481599602@f468.i.mail.ru> <1526921455.373334107@f455.i.mail.ru> From: Vladislav Shpilevoy Message-ID: <77671f74-9ee6-e05b-f317-41cf9fab7b02@tarantool.org> Date: Mon, 21 May 2018 21:26:59 +0300 MIME-Version: 1.0 In-Reply-To: <1526921455.373334107@f455.i.mail.ru> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit 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, =?UTF-8?B?0JzQtdGA0LPQtdC9INCY0LzQtdC1?= =?UTF-8?B?0LI=?= , Kirill Yukhin LGTM. Kirill, can you please take a look? On 21/05/2018 19:50, Мерген Имеев wrote: > > > > Понедельник, 21 мая 2018, 18:27 +03:00 от Vladislav Shpilevoy : > > Thanks a lot! > > But please, apply this diff: > > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > index 7a7728992..0b610dca7 100644 > --- a/src/box/sql/select.c > +++ b/src/box/sql/select.c > @@ -1994,8 +1994,10 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) >    assert(p->pOffset == 0 || p->pLimit != 0); >    if (p->pLimit) { >    if((p->pLimit->flags & EP_Collate) != 0 || > - (p->pOffset && (p->pOffset->flags & EP_Collate) != 0)) { > - sqlite3ErrorMsg(pParse, "near \"COLLATE\": syntax error"); > + (p->pOffset != NULL && > + (p->pOffset->flags & EP_Collate) != 0)) { > + sqlite3ErrorMsg(pParse, "near \"COLLATE\": "\ > + "syntax error"); >    return; >    } >    p->iLimit = iLimit = ++pParse->nMem; > > In your patch sqlite3ErrorMsg was out of 80 symbols, and pointer p->pOffset must be checked > on != NULL explicitly. (I see, that other SQLite3 code does not this, but we will fix > this gradually). > > Done. > > > > After this the patch LGTM. > > > commit 8ae83069671e7384c70523f4eda201d4815dba26 > Author: ImeevMA > Date:   Mon May 21 16:21:57 2018 +0300 > >     sql: COLLATE after LIMIT throws an error > >     Originally, SQLite3 execute queries with COLLATE after LIMIT like >     "SELECT * FROM test LIMIT N COLLATE not_exist" >     and queries without COLLATE like >     "SELECT * FROM test LIMIT N" >     the same way. > >     From this patch on queries with COLLATE after LIMIT >     or OFFSET throws a syntax error. > >     Closes #3010 > > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > index 29075d5..fc1da27 100644 > --- a/src/box/sql/select.c > +++ b/src/box/sql/select.c > @@ -1993,6 +1993,13 @@ computeLimitRegisters(Parse * pParse, Select * p, int iBreak) >      sqlite3ExprCacheClear(pParse); >      assert(p->pOffset == 0 || p->pLimit != 0); >      if (p->pLimit) { > +        if((p->pLimit->flags & EP_Collate) != 0 || > +           (p->pOffset != NULL && > +           (p->pOffset->flags & EP_Collate) != 0)) { > +            sqlite3ErrorMsg(pParse, "near \"COLLATE\": "\ > +                        "syntax error"); > +            return; > +        } >          p->iLimit = iLimit = ++pParse->nMem; >          v = sqlite3GetVdbe(pParse); >          assert(v != 0); > diff --git a/test/sql/collation.result b/test/sql/collation.result > new file mode 100644 > index 0000000..3a4f81f > --- /dev/null > +++ b/test/sql/collation.result > @@ -0,0 +1,41 @@ > +remote = require('net.box') > +--- > +... > +-- gh-3010: COLLATE after LIMIT should throw an error > +-- All of these tests should throw error "near "COLLATE": syntax error" > +box.sql.execute("SELECT 1 LIMIT 1 COLLATE BINARY;") > +--- > +- error: 'near "COLLATE": syntax error' > +... > +box.sql.execute("SELECT 1 LIMIT 1 COLLATE BINARY OFFSET 1;") > +--- > +- error: 'near "COLLATE": syntax error' > +... > +box.sql.execute("SELECT 1 LIMIT 1 OFFSET 1 COLLATE BINARY;") > +--- > +- error: 'near "COLLATE": syntax error' > +... > +box.sql.execute("SELECT 1 LIMIT 1, 1 COLLATE BINARY;") > +--- > +- error: 'near "COLLATE": syntax error' > +... > +box.sql.execute("SELECT 1 LIMIT 1 COLLATE BINARY, 1;") > +--- > +- error: 'near "COLLATE": syntax error' > +... > +box.schema.user.grant('guest','read,write,execute', 'universe') > +--- > +... > +cn = remote.connect(box.cfg.listen) > +--- > +... > +cn:execute('select 1 limit ? collate not_exist', {1}) > +--- > +- error: 'Failed to execute SQL statement: near "COLLATE": syntax error' > +... > +cn:close() > +--- > +... > +box.schema.user.revoke('guest', 'read,write,execute', 'universe') > +--- > +... > diff --git a/test/sql/collation.test.lua b/test/sql/collation.test.lua > new file mode 100644 > index 0000000..fe8c1ba > --- /dev/null > +++ b/test/sql/collation.test.lua > @@ -0,0 +1,19 @@ > +remote = require('net.box') > + > +-- gh-3010: COLLATE after LIMIT should throw an error > + > +-- All of these tests should throw error "near "COLLATE": syntax error" > +box.sql.execute("SELECT 1 LIMIT 1 COLLATE BINARY;") > +box.sql.execute("SELECT 1 LIMIT 1 COLLATE BINARY OFFSET 1;") > +box.sql.execute("SELECT 1 LIMIT 1 OFFSET 1 COLLATE BINARY;") > +box.sql.execute("SELECT 1 LIMIT 1, 1 COLLATE BINARY;") > +box.sql.execute("SELECT 1 LIMIT 1 COLLATE BINARY, 1;") > + > + > +box.schema.user.grant('guest','read,write,execute', 'universe') > +cn = remote.connect(box.cfg.listen) > + > +cn:execute('select 1 limit ? collate not_exist', {1}) > + > +cn:close() > +box.schema.user.revoke('guest', 'read,write,execute', 'universe') > >