From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Roman Khabibov <roman.habibov@tarantool.org> Subject: [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args Date: Tue, 2 Jul 2019 18:55:06 +0300 [thread overview] Message-ID: <2551430B-3DD4-4DAC-BC2B-7FA4F142A0FB@tarantool.org> (raw) In-Reply-To: <BBF3C0F8-462A-4F5B-8976-7B743DA69D24@tarantool.org> >>> @@ -4396,6 +4433,8 @@ sqlExprCodeAtInit(Parse * pParse, /* Parsing context */ >>> int >>> sqlExprCodeTemp(Parse * pParse, Expr * pExpr, int *pReg) >>> { >>> + if (pExpr->op == TK_COLLATE && check_collate_arg(pParse, pExpr) != 0) >>> + return 0; >> >> Why this check is required (in addition to the same check in ExprCodeTarget)? > It is needed e.g. for the following queries: “SELECT NOT TRUE COLLATE “unicode””. > “NOT” will be on the root. The sequence of calls: sqlExprCodeTagret -> (case TK_NOT) > -> sqlExprCodeTemp. > NOT > / \ > unicode 0x0 > / \ > TRUE 0x0 >>> int r2; >>> pExpr = sqlExprSkipCollate(pExpr); In fact, problem is here: this function skips collate token. So, diff is following: @@ -4427,10 +4426,7 @@ sqlExprCodeAtInit(Parse * pParse, /* Parsing context */ int sqlExprCodeTemp(Parse * pParse, Expr * pExpr, int *pReg) { - if (pExpr->op == TK_COLLATE && check_collate_arg(pParse, pExpr) != 0) - return 0; int r2; - pExpr = sqlExprSkipCollate(pExpr); if (ConstFactorOk(pParse) && pExpr->op != TK_REGISTER && sqlExprIsConstantNotJoin(pExpr) ) { What is more, you don’t need to set type of expression in sqlExprCodeTarget(): @@ -4201,7 +4201,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) } case TK_SPAN: case TK_COLLATE:{ - pExpr->pLeft->type = sql_expr_type(pExpr->pLeft); if (check_collate_arg(pParse, pExpr) != 0) break; return sqlExprCodeTarget(pParse, pExpr->pLeft, >>> diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua >>> index 1ca6a6446..1fdee16b7 100755 >>> --- a/test/sql-tap/in3.test.lua >>> +++ b/test/sql-tap/in3.test.lua >>> @@ -186,33 +186,6 @@ test:do_test( >>> -- </in3-1.13> >>> }) >>> >>> - >>> - >>> --- The first of these queries has to use the temp-table, because the >>> --- collation sequence used for the index on "t1.a" does not match the >>> --- collation sequence used by the "IN" comparison. The second does not >>> --- require a temp-table, because the collation sequences match. >>> --- >>> -test:do_test( >>> - "in3-1.14", >>> - function() >>> - return exec_neph(" SELECT a FROM t1 WHERE a COLLATE \"unicode_ci\" IN (SELECT a FROM t1) ") >>> - end, { >>> - -- <in3-1.14> >>> - 1, 1, 3, 5 >>> - -- </in3-1.14> >>> - }) >>> - >>> -test:do_test( >>> - "in3-1.15", >>> - function() >>> - return exec_neph(" SELECT a FROM t1 WHERE a COLLATE \"binary\" IN (SELECT a FROM t1) ") >>> - end, { >>> - -- <in3-1.15> >>> - 1, 1, 3, 5 >>> - -- </in3-1.15> >>> - }) >> >> I’d better fix these tests rather than delete. > > Previous test in this file will be same if I remove COLLATE clauses. You don’t have to remove collate clause, you should create new table with string field type and run this test using that table. Comment above the test clearly indicates its purpose.
next prev parent reply other threads:[~2019-07-02 15:55 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <cover.1561372731.git.roman.habibov@tarantool.org> [not found] ` <49f3a1279811efc66c580c6039cf1b890ff37889.1561372731.git.roman.habibov@tarantool.org> 2019-06-25 17:30 ` [tarantool-patches] Re: [PATCH 1/2 v2] test: check that collations isn't ignored in SELECTs n.pettik 2019-06-25 17:55 ` Vladislav Shpilevoy 2019-06-28 13:11 ` Roman Khabibov 2019-06-28 13:13 ` Roman Khabibov 2019-07-02 17:21 ` n.pettik [not found] ` <91edd7e8b72a93c5b5c0592c181576fca98e66fd.1561372731.git.roman.habibov@tarantool.org> 2019-06-25 17:30 ` [tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args n.pettik 2019-06-28 12:57 ` Roman Khabibov 2019-07-02 15:55 ` n.pettik [this message] 2019-07-06 1:04 ` Roman Khabibov 2019-07-08 13:38 ` n.pettik 2019-07-16 13:09 ` Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=2551430B-3DD4-4DAC-BC2B-7FA4F142A0FB@tarantool.org \ --to=korablev@tarantool.org \ --cc=roman.habibov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH 2/2 v2] sql: allow <COLLATE> only for string-like args' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox