[tarantool-patches] [PATCH v3] sql: remove support of partial indexes

[tarantool-patches] [PATCH v3] sql: remove support of partial indexes

[Redacted sender "gleb-skiba" for DMARC]

From: Gleb <gleb-skiba@mail.ru>

Remove support of partial indexes.
Add test which checks inaccessibility of partial index syntax.
Delete usage of partial index from tests.
They can be enabled after #2626.

Fixes #2165
---
Issue from https://github.com/tarantool/tarantool/issues/2165.
Source from https://github.com/tarantool/tarantool/tree/gh-2165-remove-support-partial-indexes.
Changes in v3:
 -return tests which use partial.
 -delete "WHERE" from returned tests.
 -correct syntax errors in commit

 src/box/sql/parse.y                                |  4 ++--
 test/sql-tap/analyze9.test.lua                     |  2 +-
 test/sql-tap/autoindex4.test.lua                   |  2 +-
 test/sql-tap/fkey1.test.lua                        |  2 +-
 ...gh-2165-remove-support-partial-indexes.test.lua | 17 +++++++++++++++
 test/sql-tap/index6.test.lua                       | 12 +++++------
 test/sql-tap/index7.test.lua                       | 25 +++++++++++-----------
 7 files changed, 41 insertions(+), 23 deletions(-)
 create mode 100644 test/sql-tap/gh-2165-remove-support-partial-indexes.test.lua

diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index b078e20..e0b5a2e 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1241,10 +1241,10 @@ paren_exprlist(A) ::= LP exprlist(X) RP.  {A = X;}
 ///////////////////////////// The CREATE INDEX command ///////////////////////
 //
 cmd ::= createkw(S) uniqueflag(U) INDEX ifnotexists(NE) nm(X)
-        ON nm(Y) LP sortlist(Z) RP where_opt(W). {
+        ON nm(Y) LP sortlist(Z) RP. {
   sqlite3CreateIndex(pParse, &X, 
                      sqlite3SrcListAppend(pParse->db,0,&Y), Z, U,
-                      &S, W, SQLITE_SO_ASC, NE, SQLITE_IDXTYPE_APPDEF);
+                      &S, 0, SQLITE_SO_ASC, NE, SQLITE_IDXTYPE_APPDEF);
 }
 
 %type uniqueflag {int}
diff --git a/test/sql-tap/analyze9.test.lua b/test/sql-tap/analyze9.test.lua
index 4ce575e..3668211 100755
--- a/test/sql-tap/analyze9.test.lua
+++ b/test/sql-tap/analyze9.test.lua
@@ -1111,7 +1111,7 @@ test:do_test(
         test:execsql([[
             DROP TABLE IF EXISTS t1;
             CREATE TABLE t1(id INTEGER PRIMARY KEY AUTOINCREMENT, a, b, c, d);
-            CREATE INDEX i1 ON t1(a, b) WHERE d IS NOT NULL;
+            CREATE INDEX i1 ON t1(a, b);
             INSERT INTO t1 VALUES(null, -1, -1, -1, NULL);
             INSERT INTO t1 SELECT null, 2*a,2*b,2*c,d FROM t1;
             INSERT INTO t1 SELECT null, 2*a,2*b,2*c,d FROM t1;
diff --git a/test/sql-tap/autoindex4.test.lua b/test/sql-tap/autoindex4.test.lua
index 45bae48..a567c5e 100755
--- a/test/sql-tap/autoindex4.test.lua
+++ b/test/sql-tap/autoindex4.test.lua
@@ -111,7 +111,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "autoindex4-3.1",
     [[
-        CREATE INDEX Items_x1 ON Items(ItemName,Name) WHERE ItemName = 'dummy';
+        CREATE INDEX Items_x1 ON Items(ItemName,Name);
 
         SELECT Items.ItemName
           FROM Items
diff --git a/test/sql-tap/fkey1.test.lua b/test/sql-tap/fkey1.test.lua
index 8749e1f..bca82d9 100755
--- a/test/sql-tap/fkey1.test.lua
+++ b/test/sql-tap/fkey1.test.lua
@@ -212,7 +212,7 @@ test:do_execsql_test(
     "fkey1-6.1",
     [[
         CREATE TABLE p1(id PRIMARY KEY, x, y);
-        CREATE UNIQUE INDEX p1x ON p1(x) WHERE y<2;
+        CREATE INDEX p1x ON p1(x);
         INSERT INTO p1 VALUES(1, 1, 1);
         CREATE TABLE c1(a PRIMARY KEY REFERENCES p1(x));
     ]], {
diff --git a/test/sql-tap/gh-2165-remove-support-partial-indexes.test.lua b/test/sql-tap/gh-2165-remove-support-partial-indexes.test.lua
new file mode 100644
index 0000000..d8210fc
--- /dev/null
+++ b/test/sql-tap/gh-2165-remove-support-partial-indexes.test.lua
@@ -0,0 +1,17 @@
+#!/usr/bin/env tarantool
+test = require("sqltester")
+
+test:plan(1)
+
+
+test:do_catchsql_test(
+    "partial-index-1",
+    [[
+        CREATE TABLE t1 (a INTEGER PRIMARY KEY, b INTEGER)
+        CREATE UNIQUE INDEX i ON t1 (a) WHERE a = 3;
+    ]], {
+        1, "keyword \"CREATE\" is reserved"
+    })
+
+--This test intended to be deleted in #2626
+test:finish_test()
diff --git a/test/sql-tap/index6.test.lua b/test/sql-tap/index6.test.lua
index 2aa97e8..069623f 100755
--- a/test/sql-tap/index6.test.lua
+++ b/test/sql-tap/index6.test.lua
@@ -225,7 +225,7 @@ test:do_execsql_test(
     "index6-6.0",
     [[
         CREATE TABLE t6(a,b, PRIMARY KEY (a,b));
-        CREATE INDEX t6b ON t6(b) WHERE b=1;
+        CREATE INDEX t6b ON t6(b);
         INSERT INTO t6(a,b) VALUES(123,456);
         SELECT * FROM t6;
     ]], {
@@ -262,7 +262,7 @@ test:do_execsql_test(
         CREATE TABLE t7a(id primary key, x);
         CREATE TABLE t7b(id primary key, y);
         INSERT INTO t7a VALUES(1, 1);
-        CREATE INDEX t7ax ON t7a(x) WHERE x=99;
+        CREATE INDEX t7ax ON t7a(x);
         SELECT x,y FROM t7a LEFT JOIN t7b ON (x=99) ORDER BY x;
     ]], {
         -- <index6-7.0>
@@ -318,7 +318,7 @@ test:do_execsql_test(
     [[
         CREATE TABLE t8a(id primary key, a,b);
         CREATE TABLE t8b(id primary key, x,y);
-        CREATE INDEX i8c ON t8b(y) WHERE x = 'value';
+        CREATE INDEX i8c ON t8b(y);
 
         INSERT INTO t8a VALUES(1, 1, 'one');
         INSERT INTO t8a VALUES(2, 2, 'two');
@@ -364,7 +364,7 @@ if (0 > 0)
         "index6-9.1",
         [[
             CREATE TABLE t9(a int, b int, c int);
-            CREATE INDEX t9ca ON t9(c,a) WHERE a in (10,12,20);
+            CREATE INDEX t9ca ON t9(c,a);
             INSERT INTO t9 VALUES(1,1,9),(10,2,35),(11,15,82),(20,19,5),(NULL,7,3);
             UPDATE t9 SET b=c WHERE a in (10,12,20);
             SELECT a,b,c,'|' FROM t9 ORDER BY a;
@@ -379,7 +379,7 @@ test:do_execsql_test(
     [[
         --DROP TABLE t9;
         CREATE TABLE t9(a int, b int, c int, PRIMARY KEY(a));
-        CREATE INDEX t9ca ON t9(c,a) WHERE a in (10,12,20);
+        CREATE INDEX t9ca ON t9(c,a);
         INSERT INTO t9 VALUES(1,1,9),(10,2,35),(11,15,82),(20,19,5);
         UPDATE t9 SET b=c WHERE a in (10,12,20);
         SELECT a,b,c,'|' FROM t9 ORDER BY a;
@@ -400,7 +400,7 @@ test:do_execsql_test(
           (2,3,4,5,6),
           (3,4,5,6,7),
           (1,2,3,8,9);
-        CREATE INDEX t10x ON t10(d) WHERE a=1 AND b=2 AND c=3;
+        CREATE INDEX t10x ON t10(d);
         SELECT e FROM t10 WHERE a=1 AND b=2 AND c=3 ORDER BY d;
     ]], {
         -- <index6-10.1>
diff --git a/test/sql-tap/index7.test.lua b/test/sql-tap/index7.test.lua
index c8f56eb..c194859 100755
--- a/test/sql-tap/index7.test.lua
+++ b/test/sql-tap/index7.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(5)
+test:plan(4)
 
 --!./tcltestrunner.lua
 -- 2013-11-04
@@ -261,7 +261,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "index7-6.2",
     [[
-        CREATE INDEX i4 ON t4(c) WHERE d='xyz';
+        CREATE INDEX i4 ON t4(c);
         SELECT a,b,c,d FROM (SELECT a,b FROM t5 WHERE a=1 AND b='xyz'), t4 WHERE c='abc';
     ]], {
         -- <index7-6.2>
@@ -291,14 +291,15 @@ test:do_eqp_test(
         -- </index7-6.4>
     })
 
-test:do_catchsql_test(
-    "index7-6.5",
-    [[
-        CREATE INDEX t5a ON t5(a) WHERE a=#1;
-    ]], {
-        -- <index7-6.5>
-        1, [[near "#1": syntax error]]
-        -- </index7-6.5>
-    })
-
+--test:do_catchsql_test(
+--    "index7-6.5",
+--    [[
+--        CREATE INDEX t5a ON t5(a) WHERE a=#1;
+--    ]], {
+--        -- <index7-6.5>
+--        1, [[near "#1": syntax error]]
+--        -- </index7-6.5>
+--    })
+-- this test checks incorrect syntax of partial indexes
+-- to be enabled after #2626
 test:finish_test()
-- 
2.7.4

[tarantool-patches] Re: [PATCH v3] sql: remove support of partial indexes

[Alexander Turenko]

LGTM. Nikita, can you look?

Gleb, why do you sent two v3 patches? Also there are no any list of
changes of this v3 in comparison to the previous v3. It is for you
information, no actions expected from you for now.

WBR, Alexander Turenko.

On Wed, Apr 25, 2018 at 02:00:44PM +0300, Redacted sender "gleb-skiba" for DMARC wrote:
> From: Gleb <gleb-skiba@mail.ru>
> 
> Remove support of partial indexes.
> Add test which checks inaccessibility of partial index syntax.
> Delete usage of partial index from tests.
> They can be enabled after #2626.
> 
> Fixes #2165
> ---
> Issue from https://github.com/tarantool/tarantool/issues/2165.
> Source from https://github.com/tarantool/tarantool/tree/gh-2165-remove-support-partial-indexes.
> Changes in v3:
>  -return tests which use partial.
>  -delete "WHERE" from returned tests.
>  -correct syntax errors in commit

[tarantool-patches] Re: [PATCH v3] sql: remove support of partial indexes

[n.pettik]

> LGTM. Nikita, can you look?

Hello. Generally speaking, patch looks OK to me as well.

I don’t like error message: it is possible to make it more
intelligible. To do it, you need to left parser as is, but in
sqlite3CreateIndex() check that ‘WHERE’ arg is empty.
On the other hand, this is temporary ban and to enable this
feature back, it is enough to revert this patch, so never mind.

[tarantool-patches] Re: [PATCH v3] sql: remove support of partial indexes

[Kirill Yukhin]

Hello,
On 25 апр 18:44, n.pettik wrote:
> 
> > LGTM. Nikita, can you look?
> 
> Hello. Generally speaking, patch looks OK to me as well.
> 
> I don’t like error message: it is possible to make it more
> intelligible. To do it, you need to left parser as is, but in
> sqlite3CreateIndex() check that ‘WHERE’ arg is empty.
> On the other hand, this is temporary ban and to enable this
> feature back, it is enough to revert this patch, so never mind.


New test fails for me:
/export/kyukhin/tarantool/src/test$ ./test-run.py --builddir ../../bld sql-tap/gh-2165-remove-support-partial-indexes.test.lua

======================================================================================
WORKR TEST                                            PARAMS          RESULT
---------------------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/gevent/greenlet.py", line 534, in run
    result = self._run(*self.args, **self.kwargs)
  File "/export/kyukhin/tarantool/src/test-run/lib/test.py", line 38, in _run
    self.callable(*self.callable_args, **self.callable_kwargs)
  File "/export/kyukhin/tarantool/src/test-run/lib/app_server.py", line 18, in run_server
    server.process = Popen(execs, stdout=PIPE, stderr=PIPE, cwd=cwd)
  File "/usr/lib/python2.7/dist-packages/gevent/subprocess.py", line 534, in __init__
    reraise(*exc_info)
  File "/usr/lib/python2.7/dist-packages/gevent/subprocess.py", line 503, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python2.7/dist-packages/gevent/subprocess.py", line 1253, in _execute_child
    raise child_exception
OSError: [Errno 13] Permission denied
<TestRunGreenlet at 0x7f25b3d20730 info='None'> failed with OSError

[001] sql-tap/gh-2165-remove-support-partial-indexes.test.lua
[001] TAP13 parse failed: Missing plan in the TAP source
[001] [ new ]
---------------------------------------------------------------------------------
Statistics:
* new: 1

--
Regards, Kirill Yukhin

[tarantool-patches] Re: [tarantool-patches] Re: [PATCH v3] sql: remove support of partial indexes

[GLEB SKIBA]

[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]

Hello. I fix it.


>Четверг, 26 апреля 2018, 12:02 +03:00 от Kirill Yukhin <kyukhin@tarantool.org>:
>
>Hello,
>On 25 апр 18:44, n.pettik wrote:
>> 
>> > LGTM. Nikita, can you look?
>> 
>> Hello. Generally speaking, patch looks OK to me as well.
>> 
>> I don’t like error message: it is possible to make it more
>> intelligible. To do it, you need to left parser as is, but in
>> sqlite3CreateIndex() check that ‘WHERE’ arg is empty.
>> On the other hand, this is temporary ban and to enable this
>> feature back, it is enough to revert this patch, so never mind.
>
>
>New test fails for me:
>/export/kyukhin/tarantool/src/test$ ./test-run.py --builddir ../../bld sql-tap/gh-2165-remove-support-partial-indexes.test.lua
>
>======================================================================================
>WORKR TEST                                            PARAMS          RESULT
>---------------------------------------------------------------------------------
>Traceback (most recent call last):
>  File "/usr/lib/python2.7/dist-packages/gevent/greenlet.py", line 534, in run
>    result = self._run(*self.args, **self.kwargs)
>  File "/export/kyukhin/tarantool/src/test-run/lib/test.py", line 38, in _run
>    self.callable(*self.callable_args, **self.callable_kwargs)
>  File "/export/kyukhin/tarantool/src/test-run/lib/app_server.py", line 18, in run_server
>    server.process = Popen(execs, stdout=PIPE, stderr=PIPE, cwd=cwd)
>  File "/usr/lib/python2.7/dist-packages/gevent/subprocess.py", line 534, in __init__
>    reraise(*exc_info)
>  File "/usr/lib/python2.7/dist-packages/gevent/subprocess.py", line 503, in __init__
>    restore_signals, start_new_session)
>  File "/usr/lib/python2.7/dist-packages/gevent/subprocess.py", line 1253, in _execute_child
>    raise child_exception
>OSError: [Errno 13] Permission denied
><TestRunGreenlet at 0x7f25b3d20730 info='None'> failed with OSError
>
>[001] sql-tap/gh-2165-remove-support-partial-indexes.test.lua
>[001] TAP13 parse failed: Missing plan in the TAP source
>[001] [ new ]
>---------------------------------------------------------------------------------
>Statistics:
>* new: 1
>
>--
>Regards, Kirill Yukhin
>




[-- Attachment #2: Type: text/html, Size: 3041 bytes --]

[tarantool-patches] Re: [PATCH v3] sql: remove support of partial indexes

[Alexander Turenko]

Hi Gleb!

Firstly, don't add autogenerated files like parse.c under version
control.

Add a person you want to respond you into 'To' header and others
(including mailing list) in 'Cc'. Just sending a message to the mailing
list doesn't trigger any review processes.

Send a message with open email to allow direct reply you (not through
mailing list). Here your email is shown as dmarc-noreply@freelists.org.

Please, fix the patch and ask me for review again.

WBR, Alexander Turenko.

On Tue, Apr 24, 2018 at 09:39:07PM +0300, Gleb wrote:
> Remove support of partial indexes.
> Add test which checks inaccessibility of partial index syntax.
> Delete usage of partial index from tests.
> They can be enabled after #2626.
> 
> Fixes #2165
> ---
> Issue from https://github.com/tarantool/tarantool/issues/2165.
> Source from https://github.com/tarantool/tarantool/tree/gh-2165-remove-support-partial-indexes.
> Changes in v3:
>  -return tests which use partial.
>  -delete "WHERE" from returned tests.
>  -correct syntax errors in commit