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 5173222046 for ; Mon, 21 May 2018 09:45:22 -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 WeMoSs6CD_4L for ; Mon, 21 May 2018 09:45:22 -0400 (EDT) Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94]) (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 021652202B for ; Mon, 21 May 2018 09:45:21 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 1/1] sql: COLLATE after LIMIT throws an error References: <7b33703aefcb498882fe8be652d129fcf5357e96.1526909105.git.imeevma@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Mon, 21 May 2018 16:45:19 +0300 MIME-Version: 1.0 In-Reply-To: <7b33703aefcb498882fe8be652d129fcf5357e96.1526909105.git.imeevma@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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, ImeevMA Hello. Thanks for the patch! See my 2 comments below. On 21/05/2018 16:35, ImeevMA wrote: > 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. > > Closes #3010 > --- > Branch: https://github.com/tarantool/tarantool/tree/gh-3010-COLLATE-after-LIMIT-throws-an-error > Issue: https://github.com/tarantool/tarantool/issues/3010 > > src/box/sql/select.c | 5 +++++ > test/sql-tap/collation.test.lua | 44 ++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 48 insertions(+), 1 deletion(-) > > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > index 29075d5..5e33256 100644 > --- a/src/box/sql/select.c > +++ b/src/box/sql/select.c > @@ -1993,6 +1993,11 @@ 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) || > + (p->pOffset && p->pOffset->flags & EP_Collate)) { 1. Please, use explicit != 0 to check, that an integer is not 0: if ((p->pLimit->flags & EP_Collate) != 0 ...) And fix alignment - the second line is very over indented. > diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua > index 8a98de9..5c7255a 100755 > --- a/test/sql-tap/collation.test.lua > +++ b/test/sql-tap/collation.test.lua > @@ -1,6 +1,6 @@ > #!/usr/bin/env tarantool > test = require("sqltester") > -test:plan(173) > +test:plan(177) > > local prefix = "collation-" > > @@ -249,4 +249,46 @@ local like_testcases = > > test:do_catchsql_set_test(like_testcases, prefix) > > +-- gh-3010: COLLATE after LIMIT should throw an error > + > +test:do_catchsql_test( > + "collate-after-limit-1.0", > + "SELECT 1 LIMIT 1 COLLATE BINARY;", { > + -- > + 1, "near \"COLLATE\": syntax error" > + -- > +}); > + > +test:do_catchsql_test( > + "collate-after-limit-1.1", > + "SELECT 1 LIMIT 1 OFFSET 2 COLLATE BINARY;", { > + -- > + 1, "near \"COLLATE\": syntax error" > + -- > +}); > + > +test:do_catchsql_test( > + "collate-after-limit-1.2", > + "SELECT 1 LIMIT 1 COLLATE BINARY, 2;", { > + -- > + 1, "near \"COLLATE\": syntax error" > + -- > +}); > + > + > +local net_box = require('net.box') > +local test_run = require('test_run') > +local inspector = test_run.new() 2. Hmm, it looks not to be a good idea to move netbox test there. Lets better do this: create file sql/collation.test.lua, and move all of gh-3010 tests into it. And please, add a test with OFFSET ... COLLATE. > + > +inspector:cmd("create server second with script='box/box.lua'\n") > +inspector:cmd('start server second') > +local uri = inspector:eval('second', 'box.cfg.listen')[1] > +local conn = net_box.connect(uri) > + > +test:ok(not pcall(function() conn:execute("select 1 limit 1 collate not_exist") end), > + 'attempt to use collate after limit') > + > +conn:close() > +inspector:cmd('stop server second with cleanup=1') > + > test:finish_test() >