From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 232B86EC40; Thu, 3 Jun 2021 00:04:24 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 232B86EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1622667864; bh=4zEM0s5dzPxpzgG8nXhi6EDWmFacwhcWJxKB/oZ8VZE=; h=To:Cc:References:In-Reply-To:Date:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=PmWVCKyK8CrnyOt8JrBxPBARmzu6DDNr3K3hCuYlFG/nS6+S5JV33o7dt96Q3uany JCnJAV3fmcKVQLEiX29vnUP8Jk8bsvd2CAMtzc5BXJeoa90JY3VPKWqjnQZdskWm1r aRhjquM4zzDez2DJaTXy97WME5q9g/7mVP8KQGME= Received: from smtp31.i.mail.ru (smtp31.i.mail.ru [94.100.177.91]) (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 811B06EC40 for ; Thu, 3 Jun 2021 00:04:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 811B06EC40 Received: by smtp31.i.mail.ru with esmtpa (envelope-from ) id 1loY25-0007pp-UF; Thu, 03 Jun 2021 00:04:10 +0300 To: "'Mergen Imeev'" Cc: References: <4c9c6d3f-78e7-d19e-bb54-868d3b469d13@tarantool.org> In-Reply-To: <4c9c6d3f-78e7-d19e-bb54-868d3b469d13@tarantool.org> Date: Thu, 3 Jun 2021 00:04:09 +0300 Message-ID: <052f01d757f2$d50cd890$7f2689b0$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQIN143y2CAuVXbaZAstAFvLluhQBgHR496FqoWhEZA= Content-Language: ru X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D5B0DA836B685C540E30C2BDD69416C6E46C178CD572CB10182A05F5380850407980DF0B69DAE3A682FDA28A273E0A751C0E8A9AC5DC2B7BDD5F58464DDCE306 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F2393C4755A27B53EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637FAFFEDEAEB71C4328638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D89065F0FB0E811CD5664BDE71F852F461117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAE9A1BBD95851C5BA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18618001F51B5FD3F9D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE599709FD55CB46A6E21AE983DBD7FFC1D8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE32A336C6518635091C0837EA9F3D19764C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637BBEA499411984DA1EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2368A440D3B0F6089093C9A16E5BC824A2A04A2ABAA09D25379311020FFC8D4AD494FFA51BD827F931CAB9DAFD0C48957 X-C1DE0DAB: 0D63561A33F958A5614D0B5D70E3C33E5F64B89B487301B1BB92ACFE23BE8366D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75FBC5FED0552DA851410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3454548929AF40B428C3888C10E79977BDA6E6364C509A8D58F0F2AA29D524656971A2116FBCEA4C1C1D7E09C32AA3244CE311EE0B737C7787F7FC97B95D4E1B2E35DA7DC5AF9B58C0729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj+mfSpkNmA2qb2pE4+9wbIA== X-Mailru-Sender: B5B6A6EBBD94DAD840208BF9E14C1DD267CAAAC99014A0C1199DFD4A1F61BCA3FB1A08053D4E49E71EC9E4A2C82A33BC8C24925A86E657CE0C70AEE3C9A96FBAB3D7EE8ED63280BE112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 0/3] sql: modify explicit conversion tables X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Timur Safin via Tarantool-patches Reply-To: Timur Safin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" : From: Mergen Imeev : Sent: Tuesday, June 1, 2021 5:03 PM : To: Timur Safin : Cc: tarantool-patches@dev.tarantool.org : Subject: Re: [PATCH 0/3] sql: modify explicit conversion tables :=20 : Hi! Thank you for the patch-set. See 6 comments below. :=20 : 0. In general, I want to say that I do not like your style of : commit-messages. : I think there are too many unnecessary words and constructions in it. = But : that's just my opinion. I have not seen any rules prohibiting this. =C2=AF\_(=E3=83=84)_/=C2=AF Good you realize the possibility of various styles of commit messages from different people. And that some people do have several decades = prior experience in American companies with some established English writing = skills.=20 :=20 : 1. Why "tables" in name of the patch? I see only one table. There will be another table - implicit casts. :=20 : On Tue, May 25, 2021 at 12:00:59PM +0300, Timur Safin wrote: : > : > Recent RFC "Consistent Lua/SQL types" (#6009) defined ideal explicit : > and implicit conversion tables we would like to have for all current : > and future Taranool SQL types. : > : 2. Why do you need number of pull-request here? I mean, you already = have : a name : of RFC. They are mostly equivalent, yes.=20 :=20 : > This patchset modifies explict conversion tables, and implicit : 3. Here should be "explicit" I believe. Will include implicit soon. :=20 : > conversion table to come soon. The ideal picture would be as below: : > :=20 : > | 0 | 1 | 2 | 4 | 5 | 6 | 7 | 3 | 9 |10 |11 |12 | 8 | : > +---+---+---+---+---+---+---+---+---+---+---+---+---+ : > 0. any | | | | | | | | | | | | | | : > 1. unsigned | | Y | Y | Y | Y | | | Y | | | | | Y | : > 2. string | | S | Y | S | S | S | Y | S | | | | | Y | : > 4. double | | S | Y | Y | S | | | Y | | | | | Y | : > 5. integer | | S | Y | Y | Y | | | Y | | | | | Y | : > 6. boolean | | | Y | | | Y | | | | | | | Y | : > 7. varbinary | | | Y | | | | Y | | | | | | Y | : > 3. number | | S | Y | Y | S | | | Y | | | | | Y | : > 9. decimal | | | | | | | | | | | | | | : > 10. uuid | | | | | | | | | | | | | | : > 11. array | | | | | | | | | | | | | | : > 12. map | | | | | | | | | | | | | | : > 8. scalar | | S | Y | S | S | S | S | S | | | | | Y | : > +---+---+---+---+---+---+---+---+---+---+---+---+---+ : > : 4. Why there is such strange order in table?=20 This order is from illustrations of the spec. It's too late to redraw all pictures there, but reordering here is easy. So we need to live=20 with this order forever :( : Also, I believe it makes : sense to : describe what "S", "Y" means and why some cells are empty. Yes, good point.=20 :=20 : > Please pay attention that we omit DECIMAL, UUID, SCALAR and MAP rows : > and columns, as they not yet fully supported by Tarantool SQL. Once : > their support will be landed we will modify conversion table and : > tests (which we also introducing with current patchset). : > : 5. Why not drop them if they are omitted? I mean, "modify" in name of : the letter : means that you change existing rules. Because they already visualized in the RFC, and table will significantly differ to version from spec, if we will remove all not yet implemented columns and rows (and if we would reorder them in correct=20 `enum field_type` order) If we would keep this table more or less in the same form for all of future updates in future commits for newer types, then this might produce consistent picture along the way. :=20 : 6. Please include links to issue and branch in cover-letter. Yup :=20 : > : > Timur Safin (3): : > sql: fixes for boolean expressions in explicit converstion tables : > sql: enabled ANY as target for explicit conversions : > sql: introduced explicit casts test e_casts.test.lua : > : > extra/mkkeywordhash.c | 3 +- : > src/box/sql/mem.c | 39 +--- : > src/box/sql/parse.y | 3 +- : > test/sql-tap/cse.test.lua | 12 +- : > test/sql-tap/e_casts.test.lua | 355 = ++++++++++++++++++++++++++++++++ : > test/sql-tap/e_select1.test.lua | 2 +- : > test/sql-tap/in1.test.lua | 16 +- : > test/sql-tap/keyword1.test.lua | 2 +- : > test/sql-tap/misc3.test.lua | 2 +- : > test/sql/boolean.result | 38 +--- : > test/sql/types.result | 14 +- : > 11 files changed, 390 insertions(+), 96 deletions(-) : > create mode 100755 test/sql-tap/e_casts.test.lua : > : > -- : > 2.29.2 : >