From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256
 bits)) (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id A1CC342F4AD
 for <tarantool-patches@dev.tarantool.org>;
 Fri, 26 Jun 2020 00:25:29 +0300 (MSK)
References: <20200611151853.24398-1-roman.habibov@tarantool.org>
 <20200611151853.24398-2-roman.habibov@tarantool.org>
 <e9ec1678-a48a-fe61-8ed0-bfb34f938df1@tarantool.org>
 <b1f19857-21bf-7a21-f870-aab9086a2f22@tarantool.org>
 <e1e9043a-e48f-97f8-2ec5-71791537b50d@tarantool.org>
 <81127D96-BB22-4F99-A788-047F9327A4AE@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <3e040934-d6dc-200b-5925-04879fea3e83@tarantool.org>
Date: Thu, 25 Jun 2020 23:25:27 +0200
MIME-Version: 1.0
In-Reply-To: <81127D96-BB22-4F99-A788-047F9327A4AE@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH v3 1/2] sql: use unify pattern for
 column names
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Roman Khabibov <roman.habibov@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

Hi! Thanks for the fixes!

> diff --git a/test/sql-tap/colname.test.lua b/test/sql-tap/colname.test.lua
> index caa61a07a..b5f04a2d7 100755
> --- a/test/sql-tap/colname.test.lua
> +++ b/test/sql-tap/colname.test.lua
> @@ -635,4 +635,143 @@ test:do_catchsql_test(
> +-- <ORDER BY> use the first column with name "COLUMN_1" from
> +-- column list.
> +test:do_execsql2_test(
> +    "colname-12.16",
> +    [[
> +        SELECT column_1, column_1 COLLATE "unicode_ci" FROM j_1 ORDER BY column_1;
> +    ]], {
> +        "COLUMN_1",1,"COLUMN_1",1

When there is just one row, all the sorting looks the same. So even if
it will work not by first 'column_1', you won't notice. What makes the
test not so useful. Please, make it so it would be clearly visible, that
the sorting really used 'the first column'.

Also it looks innatural that you apply "COLLATE "unicode_ci"" to numbers.
Please, use an expression, which would result into something applicable to
a number and not looking exactly the same.

For example, 'column_1 + 1' or '-column_1'. The latter option would be
especially useful to check how sorting changes, when you select

    SELECT column_1, -column_1 FROM j_1 ORDER BY column_1;

Or

    SELECT -column_1, column_1 FROM j_1 ORDER BY column_1;

If really the first matched column name is used, then the results
should be different.