From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 540DE30A34 for ; Fri, 7 Dec 2018 10:02:23 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Rv6p8vx0wQyB for ; Fri, 7 Dec 2018 10:02:23 -0500 (EST) 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 turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 591D130A32 for ; Fri, 7 Dec 2018 10:02:22 -0500 (EST) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH] sql: set names for constant fields within VIEW From: "n.pettik" In-Reply-To: <2d3f83eb-b9db-23c8-b57e-cd79c954cae2@tarantool.org> Date: Fri, 7 Dec 2018 18:02:09 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20181203220354.81845-1-korablev@tarantool.org> <2d3f83eb-b9db-23c8-b57e-cd79c954cae2@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy > On 4 Dec 2018, at 15:14, Vladislav Shpilevoy = wrote: >=20 > Hi! Thanks for the patch! >=20 > On 04/12/2018 01:03, Nikita Pettik wrote: >> If VIEW contains constant fields (e.g. CREATE VIEW v AS SELECT 1, = 'k';) >> it uses string representation of literal as a field name. In the = example >> above it would be '1' and 'k'. However, if VIEW is created using AS = VALUES >> syntax, then expressions representing constant literals lack of names >> (since span-expression is not assigned in this case). >> Lets generate names for all fields which lack names for VIEW. >> Closes #3849 >> --- >> Branch: = https://github.com/tarantool/tarantool/commits/np/gh-3849-view-constant-va= lue-v2 >> Issue: https://github.com/tarantool/tarantool/issues/3849 >=20 > I prefer this variant to parser's one. Parser fix weakens perf adding = Span > to each expr even not used for VALUES. I prefer v2 for the same reason. > The patch is ok except field name. Are you sure '__auto-fld:%d' is ok? = I > think here we should follow our usual naming policy: single '_' prefix = for > system names, use '_' instead of '-', ' ', ':'. And do not use = contractions > (as I remember we do not contract system names, but I can be wrong). >=20 > Also I propose to start fields enumeration from 1. >=20 > So my favorite name template is '_auto_field_%d', from 1 to N_fields. Ok, but I had to fix several tests to use this pattern: diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 839224131..5e80225fe 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1872,7 +1872,7 @@ sqlite3ColumnsFromExprList(Parse * parse, ExprList = * expr_list, Table *table) } } if (zName =3D=3D NULL) - zName =3D "__auto-fld"; + zName =3D "_auto_field_"; zName =3D sqlite3MPrintf(db, "%s", zName); =20 /* Make sure the column name is unique. If the name is = not unique, @@ -1885,11 +1885,11 @@ sqlite3ColumnsFromExprList(Parse * parse, = ExprList * expr_list, Table *table) int j; for (j =3D nName - 1; j > 0 && sqlite3Isdigit(zName[j]); = j--); - if (zName[j] =3D=3D ':') + if (zName[j] =3D=3D '_') nName =3D j; } zName =3D - sqlite3MPrintf(db, "%.*z:%u", nName, zName, = ++cnt); + sqlite3MPrintf(db, "%.*z_%u", nName, zName, = ++cnt); diff --git a/test/sql-tap/colname.test.lua = b/test/sql-tap/colname.test.lua index 207e7a979..4423e8ead 100755 --- a/test/sql-tap/colname.test.lua +++ b/test/sql-tap/colname.test.lua @@ -158,7 +158,7 @@ test:do_execsql2_test( SELECT * FROM v1 ORDER BY 2; ]], { -- - "A", 1, "X", 4, "A:1", 1, "B", 2, "C", 3, "X:1", 4, "Y", 5, = "Z", 6 + "A",1,"X",4,"A_1",1,"B",2,"C",3,"X_1",4,"Y",5,"Z",6 -- }) =20 @@ -168,7 +168,7 @@ test:do_execsql2_test( SELECT * FROM v2 ORDER BY 2; ]], { -- - "A", 1, "X", 4, "A:1", 11, "X:1", 14, "A:2", 1, "B", 2, "C", 3, = "X:2", 4, "Y", 5, "Z", 6, "A:3", 11, "B:1", 12, "C:1", 13, "X:3", 14, = "Y:1", 15, "Z:1", 16 + = "A",1,"X",4,"A_1",11,"X_1",14,"A_2",1,"B",2,"C",3,"X_2",4,"Y",5,"Z",6,"A_3= ",11,"B_1",12,"C_1",13,"X_3",14,"Y_1",15,"Z_1",16 -- }) =20 @@ -259,7 +259,7 @@ test:do_execsql2_test( SELECT v1.a, * FROM v1 ORDER BY 2; ]], { -- - "v1.a", 1, "A", 1, "X", 4, "A:1", 1, "B", 2, "C", 3, "X:1", 4, = "Y", 5, "Z", 6 + "v1.a",1,"A",1,"X",4,"A_1",1,"B",2,"C",3,"X_1",4,"Y",5,"Z",6 -- }) =20 @@ -269,7 +269,7 @@ test:do_execsql2_test( SELECT * FROM v2 ORDER BY 2; ]], { -- - "A", 1, "X", 4, "A:1", 11, "X:1", 14, "A:2", 1, "B", 2, "C", 3, = "X:2", 4, "Y", 5, "Z", 6, "A:3", 11, "B:1", 12, "C:1", 13, "X:3", 14, = "Y:1", 15, "Z:1", 16 + = "A",1,"X",4,"A_1",11,"X_1",14,"A_2",1,"B",2,"C",3,"X_2",4,"Y",5,"Z",6,"A_3= ",11,"B_1",12,"C_1",13,"X_3",14,"Y_1",15,"Z_1",16 -- }) =20 @@ -279,7 +279,7 @@ test:do_execsql2_test( SELECT * FROM v3 ORDER BY 2; ]], { -- - "A", 1, "X", 4, "A:1", 1, "B", 2, "C", 3, "X:1", 4, "Y", 5, = "Z", 6 + "A",1,"X",4,"A_1",1,"B",2,"C",3,"X_1",4,"Y",5,"Z",6 -- }) =20 @@ -289,7 +289,7 @@ test:do_execsql2_test( SELECT * FROM v4 ORDER BY 2; ]], { -- - "A", 1, "X", 4, "A:1", 11, "X:1", 14, "A:2", 1, "B", 2, "C", 3, = "X:2", 4, "Y", 5, "Z", 6, "A:3", 11, "B:1", 12, "C:1", 13, "X:3", 14, = "Y:1", 15, "Z:1", 16 + = "A",1,"X",4,"A_1",11,"X_1",14,"A_2",1,"B",2,"C",3,"X_2",4,"Y",5,"Z",6,"A_3= ",11,"B_1",12,"C_1",13,"X_3",14,"Y_1",15,"Z_1",16 -- }) =20 @@ -380,7 +380,7 @@ test:do_execsql2_test( SELECT * FROM v1 ORDER BY 2; ]], { -- - "V1.A", 1, "V1.X", 4, "V1.A:1", 1, "V1.B", 2, "V1.C", 3, = "V1.X:1", 4, "V1.Y", 5, "V1.Z", 6 + = "V1.A",1,"V1.X",4,"V1.A_1",1,"V1.B",2,"V1.C",3,"V1.X_1",4,"V1.Y",5,"V1.Z",= 6 -- }) =20 @@ -390,7 +390,7 @@ test:do_execsql2_test( SELECT * FROM v2 ORDER BY 2; ]], { -- - "V2.A", 1, "V2.X", 4, "V2.A:1", 11, "V2.X:1", 14, "V2.A:2", 1, = "V2.B", 2, "V2.C", 3, "V2.X:2", 4, "V2.Y", 5, "V2.Z", 6, "V2.A:3", 11, = "V2.B:1", 12, "V2.C:1", 13, "V2.X:3", 14, "V2.Y:1", 15, "V2.Z:1", 16 + = "V2.A",1,"V2.X",4,"V2.A_1",11,"V2.X_1",14,"V2.A_2",1,"V2.B",2,"V2.C",3,"V2= .X_2",4,"V2.Y",5,"V2.Z",6,"V2.A_3",11,"V2.B_1",12,"V2.C_1",13,"V2.X_3",14,= "V2.Y_1",15,"V2.Z_1",16 -- }) =20 @@ -400,7 +400,7 @@ test:do_execsql2_test( SELECT * FROM v3 ORDER BY 2; ]], { -- - "V3.A", 1, "V3.X", 4, "V3.A:1", 1, "V3.B", 2, "V3.C", 3, = "V3.X:1", 4, "V3.Y", 5, "V3.Z", 6 + = "V3.A",1,"V3.X",4,"V3.A_1",1,"V3.B",2,"V3.C",3,"V3.X_1",4,"V3.Y",5,"V3.Z",= 6 -- }) =20 @@ -410,7 +410,7 @@ test:do_execsql2_test( SELECT * FROM v4 ORDER BY 2; ]], { -- - "V4.A", 1, "V4.X", 4, "V4.A:1", 11, "V4.X:1", 14, "V4.A:2", 1, = "V4.B", 2, "V4.C", 3, "V4.X:2", 4, "V4.Y", 5, "V4.Z", 6, "V4.A:3", 11, = "V4.B:1", 12, "V4.C:1", 13, "V4.X:3", 14, "V4.Y:1", 15, "V4.Z:1", 16 + = "V4.A",1,"V4.X",4,"V4.A_1",11,"V4.X_1",14,"V4.A_2",1,"V4.B",2,"V4.C",3,"V4= .X_2",4,"V4.Y",5,"V4.Z",6,"V4.A_3",11,"V4.B_1",12,"V4.C_1",13,"V4.X_3",14,= "V4.Y_1",15,"V4.Z_1",16 -- }) =20 @@ -420,7 +420,7 @@ test:do_execsql2_test( SELECT * FROM v5 ORDER BY 2; ]], { -- - "V5.A", 1, "V5.X", 4, "V5.A:1", 1, "V5.B", 2, "V5.C", 3, = "V5.X:1", 4, "V5.Y", 5, "V5.Z", 6 + = "V5.A",1,"V5.X",4,"V5.A_1",1,"V5.B",2,"V5.C",3,"V5.X_1",4,"V5.Y",5,"V5.Z",= 6 -- }) =20 @@ -430,7 +430,7 @@ test:do_execsql2_test( SELECT * FROM v6 ORDER BY 2; ]], { -- - "V6.A", 1, "V6.X", 4, "V6.A:1", 11, "V6.X:1", 14, "V6.A:2", 1, = "V6.B", 2, "V6.C", 3, "V6.X:2", 4, "V6.Y", 5, "V6.Z", 6, "V6.A:3", 11, = "V6.B:1", 12, "V6.C:1", 13, "V6.X:3", 14, "V6.Y:1", 15, "V6.Z:1", 16 + = "V6.A",1,"V6.X",4,"V6.A_1",11,"V6.X_1",14,"V6.A_2",1,"V6.B",2,"V6.C",3,"V6= .X_2",4,"V6.Y",5,"V6.Z",6,"V6.A_3",11,"V6.B_1",12,"V6.C_1",13,"V6.X_3",14,= "V6.Y_1",15,"V6.Z_1",16 -- }) > Also why don't you add number suffix to a first name? Maybe it is = worth > naming all fields in the same way? IMHO it looks more clear when first member comes as is (i.e. without ordinal number).=20