[tarantool-patches] Re: [PATCH 1/2] sql: fix collation node duplication in AST
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sun Jun 2 20:09:40 MSK 2019
Hi! Thanks for the fixes! See 2 comments below.
On 28/05/2019 17:10, Roman Khabibov wrote:
> Hi! Thanks for the review.
>
>> I vote for the second as the simplest, and add a test provided by me
>> above to ensure we will never break it accidentally.
>>
>> This commit should consist of the test and a comment in resolveAlias.
>> And keep this nice ASCII schema.
>
> diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
> index 504096e6d..f952d8c04 100644
> --- a/src/box/sql/resolve.c
> +++ b/src/box/sql/resolve.c
> @@ -109,6 +109,11 @@ resolveAlias(Parse * pParse, /* Parsing context */
> return;
> if (zType[0] != 'G')
> incrAggFunctionDepth(pDup, nSubquery);
> + /*
> + * If there was typed more than one explicit collations in
> + * query, it will be a sequence of left nodes with the
> + * collations in a tree.
> + */
1. Please, add a comment, that there is nothing special about
keeping the sequence. Only one collation could be stored, but
the present solution is simpler.
> if (pExpr->op == TK_COLLATE) {
> pDup =
> sqlExprAddCollateString(pParse, pDup, pExpr->u.zToken);
> diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua
> index 0bf54576d..3c5d3053a 100755
> --- a/test/sql-tap/collation.test.lua
> +++ b/test/sql-tap/collation.test.lua
> @@ -529,4 +529,21 @@ test:do_catchsql_test(
> 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))',
> {1, "Collation 'FOO' does not exist"})
>
> +-- gh-3805 Check that collation is not ignored. Must pass.
2. Of course it must pass. It is the purpose of test. Please,
drop 'Must pass'.
> +
> +test:do_execsql_test(
> + "collation-2.6.0",
> + [[ CREATE TABLE a (id INT PRIMARY KEY, s TEXT) ]],
> + {})
> +
> +test:do_execsql_test(
> + "collation-2.6.1",
> + [[ INSERT INTO a VALUES (1, 'B'), (2, 'b') ]],
> + {})
> +
> +test:do_execsql_test(
> + "collation-2.6.2",
> + [[ SELECT s COLLATE "unicode_ci" FROM a ORDER BY s COLLATE "unicode" ]],
> + {"b","B"})
> +
> test:finish_test()
>
More information about the Tarantool-patches
mailing list