From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Roman Khabibov <roman.habibov@tarantool.org>, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v3 1/2] sql: use unify pattern for column names Date: Mon, 15 Jun 2020 23:59:21 +0200 [thread overview] Message-ID: <e9ec1678-a48a-fe61-8ed0-bfb34f938df1@tarantool.org> (raw) In-Reply-To: <20200611151853.24398-2-roman.habibov@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 <VALUES> > 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 <VALUES> > 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.
next prev parent reply other threads:[~2020-06-15 21:59 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-11 15:18 [Tarantool-patches] [PATCH v3 0/2] Use-unify-pattern-for-column-names Roman Khabibov 2020-06-11 15:18 ` [Tarantool-patches] [PATCH v3 1/2] sql: use unify pattern for column names Roman Khabibov 2020-06-15 21:59 ` Vladislav Shpilevoy [this message] 2020-06-22 21:14 ` roman 2020-06-23 23:12 ` Vladislav Shpilevoy 2020-06-25 14:35 ` Roman Khabibov 2020-06-25 21:25 ` Vladislav Shpilevoy 2020-06-27 11:50 ` Roman Khabibov 2020-06-29 20:08 ` Vladislav Shpilevoy 2020-06-29 23:46 ` Roman Khabibov 2020-06-30 21:23 ` Vladislav Shpilevoy 2020-07-01 12:45 ` Roman Khabibov 2020-07-01 21:25 ` Vladislav Shpilevoy 2020-07-02 15:55 ` Roman Khabibov 2020-06-11 15:18 ` [Tarantool-patches] [PATCH v3 2/2] sql: print span more properly Roman Khabibov 2020-06-15 22:18 ` Vladislav Shpilevoy 2020-06-22 21:14 ` roman 2020-06-23 23:12 ` Vladislav Shpilevoy 2020-07-02 15:55 ` Roman Khabibov 2020-07-02 19:06 ` Nikita Pettik
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=e9ec1678-a48a-fe61-8ed0-bfb34f938df1@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=roman.habibov@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v3 1/2] sql: use unify pattern for column names' \ /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