From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41]) (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 E96174429E1 for ; Tue, 16 Jun 2020 00:59:23 +0300 (MSK) References: <20200611151853.24398-1-roman.habibov@tarantool.org> <20200611151853.24398-2-roman.habibov@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Mon, 15 Jun 2020 23:59:21 +0200 MIME-Version: 1.0 In-Reply-To: <20200611151853.24398-2-roman.habibov@tarantool.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Tarantool-patches] [PATCH v3 1/2] sql: use unify pattern for column names List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov , tarantool-patches@dev.tarantool.org Hi! Thanks for the patch! See 4 comments below. On 11/06/2020 17:18, Roman Khabibov wrote: > sql: use unify pattern for column names 1. 'Unify' is a verb. You don't need 'use' before it. > Name resulting columns generated by an expression or > construction by the "COLUMN_N" pattern. > > Closes #3962 > > @TarantoolBot document > Title: Column naming in SQL > > Now, every auto generated column is named by the "COLUMN_N" > pattern, where N is the number of column in a query (starting) > from 1). Auto generated column is a column in a query result > generated by an expression or a column from > construction. > > Examples: > box.execute("VALUES(1, 2, 3);") > --- > - metadata: > - name: COLUMN_1 > type: integer > - name: COLUMN_2 > type: integer > - name: COLUMN_3 > type: integer > rows: > - [1, 2, 3] > ... > box.execute("SELECT * FROM (VALUES (1+1, 1+1));") > --- > - metadata: > - name: COLUMN_1 > type: integer > - name: COLUMN_2 > type: integer > rows: > - [2, 2] > ... > box.execute("SELECT 1+1, 1+1;") > --- > - metadata: > - name: COLUMN_1 > type: integer > - name: COLUMN_2 > type: integer > rows: > - [2, 2] > ... 2. Please, wrap code into ``` to make the github markdown more readable. 3. What if I specify first column by myself, and the second is an expression? Will it be _1 or _2? > @@ -4951,6 +4951,18 @@ selectExpander(Walker * pWalker, Select * p) > || (pE->pLeft != 0 && pE->pLeft->op == TK_ID)); > if (pE->op == TK_DOT && pE->pRight->op == TK_ASTERISK) > break; > + /* > + * It is necessary to generate auto names for > + * expressions before clauses such as ORDER BY, > + * GROUP BY will be resolved. These names can be > + * found inside these clauses during resolving. > + */ > + if (pEList->a[k].zName == NULL && pE->op != TK_DOT && > + ((pE->flags & EP_Leaf) == 0 || pE->op != TK_ID)) { > + uint32_t idx = ++pParse->autoname_i; > + pEList->a[k].zName = > + sqlDbStrDup(db, sql_generate_column_name(idx)); 4. I don't think I understand what is happening here. Looks too complicated. Why do you need so many checks, what do these checks mean, how are they related to ORDER BY/GROUP BY? And how is it related to select expansion (the function is called selectExpander())? Why does it work fine when I use normal column names in ORDER BY/GROUP BY, and does not work here without this crutch? Also, this looks broken: tarantool> box.execute('SELECT *, s1 COLLATE "unicode_ci" FROM j ORDER BY column_1;') --- - null - Can’t resolve field 'COLUMN_1' ... tarantool> box.execute('SELECT s1 COLLATE "unicode_ci" FROM j ORDER BY column_1;') --- - metadata: - name: COLUMN_1 type: scalar rows: - [1] ... When I add '*' before the column_1, it stops working.