From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (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 7440541D952 for ; Tue, 30 Jun 2020 02:46:09 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3594.4.19\)) From: Roman Khabibov In-Reply-To: Date: Tue, 30 Jun 2020 02:46:07 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <0B6AD22C-DC0E-441C-B84E-9667A0AED2C3@tarantool.org> References: <20200611151853.24398-1-roman.habibov@tarantool.org> <20200611151853.24398-2-roman.habibov@tarantool.org> <81127D96-BB22-4F99-A788-047F9327A4AE@tarantool.org> <3e040934-d6dc-200b-5925-04879fea3e83@tarantool.org> 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: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the review. > On Jun 29, 2020, at 23:08, Vladislav Shpilevoy = wrote: >=20 > Hi! Thanks for the patch! >=20 >> In my patch, I used the zName field of the struct ExprList_item, >> which is also used for aliases. But I did not find words in the >> code that this field is only for aliases ( clause), therefore >> I considered it legitimate to put auto names in it. >>=20 >> PostgreSQL would throw an error in the case above: "ambiguous = names=E2=80=9D. >> But it works for us. Is it a bug or not? Perhaps yes. In any case, >> this does not apply to the patch and requires a separate discussion, >> so I just delete this test. >=20 > I wouldn't consider it a bug. And it is totally not related to this > patchset. So I would leave it as is now. >=20 > Please, don't delete any tests unless they are duplicate, or useless. >=20 > See 2 comments below. >=20 >> commit ed196a4446177eb14aa1b86a382c32416edb5794 >> Author: Roman Khabibov >> Date: Thu Mar 5 12:48:58 2020 +0300 >>=20 >> sql: unify pattern for column names >>=20 >> Name resulting columns generated by an expression or >> construction by the "COLUMN_N" pattern. >>=20 >> Closes #3962 >>=20 >> @TarantoolBot document >> Title: Column naming in SQL >>=20 >> Now, every auto generated column is named by the "COLUMN_N" >> pattern, where N is the number of generated 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. >>=20 >> 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] >> ... >> ``` >>=20 >> Here, the expression "mycol + 1" generates a new column, so that >> it is the first auto generated resulting column will be named as >> "COLUMN_1". >> ``` >> tarantool> CREATE TABLE test (mycol INT PRIMARY KEY); >> --- >> - row_count: 1 >> ... >>=20 >> tarantool> SELECT mycol, mycol + 1 FROM test; >> --- >> - metadata: >> - name: MYCOL >> type: integer >> - name: COLUMN_1 >> type: integer >> rows: [] >> ... >> ``` >> Note that you can use generated names already within the query, >> e.g. in clause. >> ``` >> tarantool> SELECT mycol, mycol + 1 FROM test ORDER BY column_1; >> --- >> - metadata: >> - name: MYCOL >> type: integer >> - name: COLUMN_1 >> type: integer >> rows: [] >> ... >> ``` >>=20 >> It should also be noted that if you use column names similar to >> the "COLUMN_N" pattern, you can get the same names as a result: >>=20 >> ``` >> tarantool> CREATE TABLE test (column_1 SCALAR PRIMARY KEY); >> --- >> - row_count: 1 >> ... >>=20 >> tarantool> INSERT INTO test VALUES(1); >> --- >> - row_count: 1 >> ... >>=20 >> tarantool> SELECT column_1, column_1 COLLATE "unicode_ci" FROM = test; >=20 > 1. In my only comment to the previous patch I asked not to use > COLLATE for numbers, and make the example more representative. > Here if I remove 'COLLATE "unicode_ci"', the result won't change. > That makes the example close to useless. >=20 > Please, reconsider my comment from the previous email. Yes. =E2=80=98colname + 1=E2=80=99 seems more logical. >> --- >> - metadata: >> - name: COLUMN_1 >> type: scalar >> - name: COLUMN_1 >> type: scalar >> rows: >> - [1, 1] >> ... >> ``` >>=20 >> diff --git a/test/sql-tap/colname.test.lua = b/test/sql-tap/colname.test.lua >> index caa61a07a..f27ffe413 100755 >> --- a/test/sql-tap/colname.test.lua >> +++ b/test/sql-tap/colname.test.lua >> @@ -635,4 +635,133 @@ test:do_catchsql_test(> + >> +test:do_execsql2_test( >> + "colname-12.14", >> + [[ >> + CREATE TABLE j_1 (column_1 SCALAR PRIMARY KEY, column_2 = SCALAR); >> + INSERT INTO j_1 VALUES(1, 1); >> + ]], {}) >> + >> +test:do_execsql2_test( >> + "colname-12.15", >> + [[ >> + SELECT column_1, column_1 COLLATE "unicode_ci", column_2, 1 = FROM j_1; >> + ]], { >> + "COLUMN_1",1,"COLUMN_1",1,"COLUMN_2",1,"COLUMN_2",1 >=20 > 2. Ditto. +test:do_execsql2_test( + "colname-12.5", + [[ + CREATE TABLE j (s1 SCALAR PRIMARY KEY); + INSERT INTO j VALUES(1); + ]], {}) + +-- +-- Column named as 'COLUMN_1', because 's1 + 1' is a expression. +-- +test:do_execsql2_test( + "colname-12.6", + [[ + SELECT s1 + 1 FROM j; + ]], { + "COLUMN_1",2 + }) + +test:do_execsql2_test( + "colname-12.7", + [[ + SELECT s1 + 1 FROM j ORDER BY column_1; + ]], { + "COLUMN_1",2 + }) + +test:do_execsql2_test( + "colname-12.8", + [[ + SELECT * FROM (SELECT s1 + 1 FROM j + ORDER BY column_1) ORDER BY column_1; + ]], { + "COLUMN_1",2 + }) + +test:do_execsql2_test( + "colname-12.9", + [[ + SELECT s1 + 1 FROM j GROUP BY column_1; + ]], { + "COLUMN_1",2 + }) + +test:do_execsql2_test( + "colname-12.10", + [[ + SELECT * FROM (SELECT s1 + 1 FROM j + ORDER BY column_1) GROUP BY column_1; + ]], { + "COLUMN_1",2 + }) + +test:do_execsql2_test( + "colname-12.11", + [[ + SELECT * FROM (SELECT s1 + 1 FROM j + ORDER BY column_1) WHERE column_1 =3D 2; + ]], { + "COLUMN_1",2 + }) + +test:do_execsql2_test( + "colname-12.12", + [[ + SELECT *, s1 + 1 FROM j ORDER BY column_1; + ]], { + "S1",1,"COLUMN_1",2 + }) + +test:do_execsql2_test( + "colname-12.13", + [[ + SELECT s1 + 1, * FROM j ORDER BY column_1; + ]], { + "COLUMN_1",2,"S1",1 + }) + +test:do_execsql2_test( + "colname-12.14", + [[ + CREATE TABLE j_1 (column_1 SCALAR PRIMARY KEY, column_2 = SCALAR); + INSERT INTO j_1 VALUES(1, 1); + ]], {}) + +test:do_execsql2_test( + "colname-12.15", + [[ + SELECT column_1, column_1 + 1, column_2, 2 FROM j_1; + ]], { + "COLUMN_1",1,"COLUMN_1",2,"COLUMN_2",1,"COLUMN_2",2 + }) + >> + }) >> + commit 6ec3cfd814229f81d12ab77ac92bdb986d91e958 Author: Roman Khabibov Date: Thu Mar 5 12:48:58 2020 +0300 sql: unify pattern for column names =20 Name resulting columns generated by an expression or construction by the "COLUMN_N" pattern. =20 Closes #3962 =20 @TarantoolBot document Title: Column naming in SQL =20 Now, every auto generated column is named by the "COLUMN_N" pattern, where N is the number of generated 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. =20 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] ... ``` =20 Here, the expression "mycol + 1" generates a new column, so that it is the first auto generated resulting column will be named as "COLUMN_1". ``` tarantool> CREATE TABLE test (mycol INT PRIMARY KEY); --- - row_count: 1 ... =20 tarantool> SELECT mycol, mycol + 1 FROM test; --- - metadata: - name: MYCOL type: integer - name: COLUMN_1 type: integer rows: [] ... ``` Note that you can use generated names already within the query, e.g. in clause. ``` tarantool> SELECT mycol, mycol + 1 FROM test ORDER BY column_1; --- - metadata: - name: MYCOL type: integer - name: COLUMN_1 type: integer rows: [] ... ``` =20 It should also be noted that if you use column names similar to the "COLUMN_N" pattern, you can get the same names as a result: =20 ``` tarantool> CREATE TABLE test (column_1 SCALAR PRIMARY KEY); --- - row_count: 1 ... =20 tarantool> INSERT INTO test VALUES(1); --- - row_count: 1 ... =20 tarantool> SELECT column_1, column_1 + 1 FROM test; --- - metadata: - name: COLUMN_1 type: scalar - name: COLUMN_1 type: scalar rows: - [1, 2] ... ``` diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 4b069addb..26c735ed7 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1854,14 +1854,14 @@ generate_column_metadata(struct Parse *pParse, = struct SrcList *pTabList, } } else { const char *z =3D NULL; - if (colname !=3D NULL) + if (colname !=3D NULL) { z =3D colname; - else if (span !=3D NULL) - z =3D span; - else - z =3D tt_sprintf("column%d", i + 1); + } else { + uint32_t idx =3D ++pParse->autoname_i; + z =3D sql_generate_column_name(idx); + } vdbe_metadata_set_col_name(v, i, z); - if (is_full_meta && colname !=3D NULL) + if (is_full_meta) vdbe_metadata_set_col_span(v, i, span); } } @@ -1897,7 +1897,6 @@ sqlColumnsFromExprList(Parse * parse, ExprList * = expr_list, /* Database connection */ sql *db =3D parse->db; u32 cnt; /* Index added to make the name unique = */ - Expr *p; /* Expression for a single result column = */ char *zName; /* Column name */ int nName; /* Size of name in zName[] */ Hash ht; /* Hash table of column names */ @@ -1929,13 +1928,12 @@ sqlColumnsFromExprList(Parse * parse, ExprList * = expr_list, space_def->field_count =3D column_count; =20 for (uint32_t i =3D 0; i < column_count; i++) { - /* Get an appropriate name for the column + /* + * Check if the column contains an "AS " + * phrase. */ - p =3D sqlExprSkipCollate(expr_list->a[i].pExpr); - if ((zName =3D expr_list->a[i].zName) !=3D 0) { - /* If the column contains an "AS " phrase, = use as the name */ - } else { - Expr *pColExpr =3D p; /* The expression that = is the result column name */ + if ((zName =3D expr_list->a[i].zName) =3D=3D 0) { + struct Expr *pColExpr =3D expr_list->a[i].pExpr; struct space_def *space_def =3D NULL; while (pColExpr->op =3D=3D TK_DOT) { pColExpr =3D pColExpr->pRight; @@ -1951,14 +1949,14 @@ sqlColumnsFromExprList(Parse * parse, ExprList * = expr_list, } else if (pColExpr->op =3D=3D TK_ID) { assert(!ExprHasProperty(pColExpr, = EP_IntValue)); zName =3D pColExpr->u.zToken; - } else { - /* Use the original text of the column = expression as its name */ - zName =3D expr_list->a[i].zSpan; } } - if (zName =3D=3D NULL) - zName =3D "_auto_field_"; - zName =3D sqlMPrintf(db, "%s", zName); + if (zName =3D=3D NULL) { + uint32_t idx =3D ++parse->autoname_i; + zName =3D sqlDbStrDup(db, = sql_generate_column_name(idx)); + } else { + zName =3D sqlDbStrDup(db, zName); + } =20 /* Make sure the column name is unique. If the name is = not unique, * append an integer to the name so that it becomes = unique. @@ -4792,6 +4790,24 @@ selectPopWith(Walker * pWalker, Select * p) } } =20 +/** + * Determine whether to generate a name for @a expr or not. + * + * Auto generated names is needed for every item in a