<HTML><BODY><br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Пятница, 18 мая 2018, 14:55 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:<br><br><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15266445130000000721_BODY">Hello. Thanks for the patch! It is almost perfect! But look at my 5 comments below.<br><br>
On 18/05/2018 14:26, Мерген Имеев wrote:<br>
> From aa5528cfdd54def0a23c6fd8223d283fc834a1d6 Mon Sep 17 00:00:00 2001<br>
> Message-Id: <<a href="mailto:aa5528cfdd54def0a23c6fd8223d283fc834a1d6.1526478195.git.imeevma@tarantool.org">aa5528cfdd54def0a23c6fd8223d283fc834a1d6.1526478195.git.imeevma@tarantool.org</a>><br>
> From: Mergen Imeev <<a href="mailto:imeevma@tarantool.org">imeevma@tarantool.org</a>><br>
> Date: Wed, 16 May 2018 16:39:26 +0300<br><br>
1. Why do I see this in the message body? How do you send mail? Manually copy-pasting into<br>
your mail client? Or via git send-mail? Please, use git send-mail. SMTP headers must not be<br>
visible in the message body.</div></div></div></div></blockquote>Understood. Will do next time.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15266445130000000721_BODY"><br><br>
> Subject: [PATCH 1/1] sql: IS is only applicable when dealing with NULL<br>
> <br>
> According to ANSI Standard IS/IS NOT can be used to determine<br>
> if values is null. At the same time in SQLite3 IS/IS NOT have an<br>
> additional function - it can be used to check equality of two<br>
> values. This patch removes that additional function.<br>
> <br>
> Closes #2136<br>
> ---<br>
> Branch: <a href="https://github.com/tarantool/tarantool/tree/gh-2136-behaviour-of-IS-corrected" target="_blank">https://github.com/tarantool/tarantool/tree/gh-2136-behaviour-of-IS-corrected</a><br>
> Issue: <a href="https://github.com/tarantool/tarantool/issues/2136" target="_blank">https://github.com/tarantool/tarantool/issues/2136</a><br>
> <br>
> src/box/sql/parse.y | 30 +---<br>
> test/sql-tap/e_expr.test.lua | 366 ++++++++++++++------------------------<br>
> test/sql-tap/lua_sql.test.lua | 12 +-<br>
> test/sql-tap/null.test.lua | 89 ++++++++-<br>
> test/sql-tap/subselect.test.lua | 2 +-<br>
> test/sql-tap/transitive1.test.lua | 45 +----<br>
> test/sql-tap/types.test.lua | 2 +-<br>
> test/sql-tap/types2.test.lua | 2 +-<br>
> 8 files changed, 229 insertions(+), 319 deletions(-)<br>
> <br>
> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y<br>
> index 872647d..4ad5195 100644<br>
> --- a/src/box/sql/parse.y<br>
> +++ b/src/box/sql/parse.y<br>
> @@ -1032,36 +1032,10 @@ expr(A) ::= expr(A) likeop(OP) expr(Y) ESCAPE expr(E). [LIKE_KW] {<br>
> }<br>
> }<br>
> <br>
> -expr(A) ::= expr(A) ISNULL|NOTNULL(E). {spanUnaryPostfix(pParse,@E,&A,&E);}<br>
> +expr(A) ::= expr(A) IS NULL(E). {spanUnaryPostfix(pParse,TK_ISNULL,&A,&E);}<br>
> +expr(A) ::= expr(A) IS NOT NULL(E). {spanUnaryPostfix(pParse,TK_NOTNULL,&A,&E);}<br>
> expr(A) ::= expr(A) NOT NULL(E). {spanUnaryPostfix(pParse,TK_NOTNULL,&A,&E);}<br><br>
2. What is 'NOT NULL' with no 'IS'. Is it allowed by the standard? Lets remove it too,<br>
if not.</div></div></div></div></blockquote>Done<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15266445130000000721_BODY"><br><br>
> <br>
> -%include {<br>
> - /* A routine to convert a binary TK_IS or TK_ISNOT expression into a<br>
> - ** unary TK_ISNULL or TK_NOTNULL expression. */<br><br>
3. I still can grep ISNULL token. NOTNULL too. They must be removed completely, but<br>
very accurately. For example, TK_NULL, TK_ISNULL, TK_NOTNULL are not tokens, they<br>
are identifiers for 'NULL', 'IS NULL', and 'IS NOT NULL'. But strings 'ISNULL' and<br>
'NOTNULL' must be removed. For example, look extra/mkkeywordhash.c:196. Here all the<br>
tokens are stored and their identifiers, so "NOTNULL" is reflected into TK_NOTNULL.<br>
We must remove "NOTNULL". Same in parse.y:137, 213.<br><br>
In treeview.c:475 NOTNULL must be replaced with "NOT NULL". Same in vdbeaux.c:1508.<br><br>
Same about ISNULL.</div></div></div></div></blockquote>Partially done. Ones in parse.y used to generate identifiers to operations 'IS NULL' and 'IS NOT NULL'<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15266445130000000721_BODY"><br><br>
> - static void binaryToUnaryIfNull(Parse *pParse, Expr *pY, Expr *pA, int op){<br>
> - sqlite3 *db = pParse->db;<br>
> - if( pA && pY && pY->op==TK_NULL ){<br>
> - pA->op = (u8)op;<br>
> - sql_expr_free(db, pA->pRight, false);<br>
> - pA->pRight = 0;<br>
> - }<br>
> - }<br>
> -}<br>
> -<br>
> -// expr1 IS expr2<br>
> -// expr1 IS NOT expr2<br>
> -//<br>
> -// If expr2 is NULL then code as TK_ISNULL or TK_NOTNULL. If expr2<br>
> -// is any other expression, code as TK_IS or TK_ISNOT.<br>
> -//<br>
> -expr(A) ::= expr(A) IS expr(Y). {<br>
> - spanBinaryExpr(pParse,TK_IS,&A,&Y);<br>
> - binaryToUnaryIfNull(pParse, Y.pExpr, A.pExpr, TK_ISNULL);<br>
> -}<br>
> -expr(A) ::= expr(A) IS NOT expr(Y). {<br>
> - spanBinaryExpr(pParse,TK_ISNOT,&A,&Y);<br>
> - binaryToUnaryIfNull(pParse, Y.pExpr, A.pExpr, TK_NOTNULL);<br>
> -}<br>
> <br>
> %include {<br>
> /* Construct an expression node for a unary prefix operator<br>
> diff --git a/test/sql-tap/e_expr.test.lua b/test/sql-tap/e_expr.test.lua<br>
> index d0f6895..d378222 100755<br>
> --- a/test/sql-tap/e_expr.test.lua<br>
> +++ b/test/sql-tap/e_expr.test.lua<br>
> @@ -512,25 +512,27 @@ test:do_execsql_test(<br>
> -- </e_expr-8.1.2><br>
> })<br>
> <br>
> -test:do_execsql_test(<br>
> - "e_expr-8.1.3",<br>
> - [[<br>
> - SELECT NULL IS 'ab'<br>
> - ]], {<br>
> - -- <e_expr-8.1.3><br>
> - 0<br>
> - -- </e_expr-8.1.3><br>
> - })<br>
> -<br>
> -test:do_execsql_test(<br>
> - "e_expr-8.1.4",<br>
> - [[<br>
> - SELECT 'ab' IS 'ab'<br>
> - ]], {<br>
> - -- <e_expr-8.1.4><br>
> - 1<br>
> - -- </e_expr-8.1.4><br>
> - })<br>
> +-- gh-2136: According to ANSI SQL IS can be used only in IS NULL or IS NOT NULL<br><br>
4. Please, remove the old tests, not comment them.</div></div></div></div></blockquote>Done<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15266445130000000721_BODY"><br>
> diff --git a/test/sql-tap/transitive1.test.lua b/test/sql-tap/transitive1.test.lua<br>
> index bdb9e97..ed3238f 100755<br>
> --- a/test/sql-tap/transitive1.test.lua<br>
> +++ b/test/sql-tap/transitive1.test.lua<br>
> @@ -234,36 +221,6 @@ test:do_execsql_test(<br>
> -- </transitive1-400><br>
> })<br>
> <br>
> -test:do_execsql_test(<br>
> - "transitive1-401",<br>
> - [[<br>
> - SELECT '1-row' FROM t401 LEFT JOIN t402 ON b IS a JOIN t403 ON c=a;<br>
> - ]], {<br>
> - -- <transitive1-401><br>
> - "1-row"<br>
> - -- </transitive1-401><br>
> - })<br><br>
5. Are you sure these tests must be removed? Looks like they test<br>
JOINs, not IS. Maybe it is better to fix them replacing IS with '=' ?</div></div></div></div></blockquote>There were four of pretty much identical tests. I deleted ones that used 'IS' as replacement to '='. One left<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15266445130000000721_BODY"><br><br>
> -<br>
> -test:do_execsql_test(<br>
> - "transitive1-402",<br>
> - [[<br>
> - SELECT '1-row' FROM t401 LEFT JOIN t402 ON b=a JOIN t403 ON c IS a;<br>
> - ]], {<br>
> - -- <transitive1-402><br>
> - "1-row"<br>
> - -- </transitive1-402><br>
> - })<br>
> -<br>
> -test:do_execsql_test(<br>
> - "transitive1-403",<br>
> - [[<br>
> - SELECT '1-row' FROM t401 LEFT JOIN t402 ON b IS a JOIN t403 ON c IS a;<br>
> - ]], {<br>
> - -- <transitive1-403><br>
> - "1-row"<br>
> - -- </transitive1-403><br>
> - })<br>
> -<br><br></div></div></div></div></blockquote>
<br>Result of "git show" contains more than 14 000 lines of text.<br></BODY></HTML>