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 2C3276EC58; Sat, 26 Jun 2021 00:27:02 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2C3276EC58 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1624656422; bh=lvgCaSlKzvnXO6+dr/OhGBXwDnAWYFyKnHpwgB1ou2o=; h=To:Cc:Date:Subject:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=JHERx4mmLhZwRyFdPBjau8tbR598VqUEhMS+u2GPOBazWk9n8W+ZG91TzUd446b8M lX0PbrftiDop+Kp2kj3MbQIq8Gp8NNU5nAZe3zZO5sf7tsI2WmiqxCiwvHj4IyIpRd 6C6x96yLQl0jNrAOBFeEFSx3+688/EqHy44ySqjc= Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (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 354246EC58 for ; Sat, 26 Jun 2021 00:27:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 354246EC58 Received: by smtp45.i.mail.ru with esmtpa (envelope-from ) id 1lwtLn-00070X-GQ; Sat, 26 Jun 2021 00:26:59 +0300 To: "'Mergen Imeev'" Cc: Date: Sat, 26 Jun 2021 00:26:58 +0300 Message-ID: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_13DD_01D76A21.F98F44A0" X-Mailer: Microsoft Outlook 16.0 X-MS-TNEF-Correlator: 000000008BA226595235B84EA11C3D5145BB9D130700C3B68E10F77511CEB4CD00AA00BBB6E600000000000D00004A08823A88D2F6458DE0EDA83EB00A460000000028DD0000 Expiry-Date: Mon, 28 Jun 2021 00:26:55 +0300 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD954DFF1DC42D673FB4F75AC5594ACDC16869A51A860A12816182A05F5380850407AC23C885DC06DC7212331935801996656B6B61AE664AD734DA426DDD6E2CE4E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F9D3BE5B596754B8C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7FEAC828D2BF6EC3CEA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BD6CF32B5F8F9D40425ADAA1315F1E3075CC90260B8AEEA9ECC7F00164DA146DAFE8445B8C89999728AA50765F7900637BA939FD1B3BAB99B389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8B861051D4BA689FCF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947CAE2BFB9A60527F4F03F1AB874ED890284AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C31CDA116245DA0C70BA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7325B7099C10CC3D7731C566533BA786AA5CC5B56E945C8DA X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2AD77751E876CB595E8F7B195E1C978312E1BDAB0D52C1AC3B59FADFEE23B5201 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8183A4AFAF3EA6BDC44C22C572F31DE52E4DFD85F345618D568F514DA0AC48057A69195EAE24681D4D99C2B6934AE262D3EE7EAB7254005DCED7532B743992DF240BDC6A1CF3F042BAD6DF99611D93F60EFE9458D376F2992EE699F904B3F4130E343918A1A30D5E7FCCB5012B2E24CD356 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3407FE5477D6A8AF081C9CDD438D1B90B87FAB3C37E4965B02054AFDAA754956534A53B61F1C45C3FD1D7E09C32AA3244CCEB2018CDC2D14B2EE15A16E64416E393FD9C8CA1B0515E0729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXjGhU4iD9b2Cbyft30OKhco X-Mailru-Sender: B5B6A6EBBD94DAD837E14476885DCAD6FAB2D8A05B5841A5B4F76C5941B6FA221EEAAD3A7B8453CB1EC9E4A2C82A33BC8C24925A86E657CE0C70AEE3C9A96FBAB3D7EE8ED63280BE112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: [Tarantool-patches] =?cp1251?b?zvLn++I6IFtQQVRDSCB2MiAyLzNdIHNx?= =?cp1251?q?l=3A_updated_explicit_conversion_table?= 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" This is a multipart message in MIME format. ------=_NextPart_000_13DD_01D76A21.F98F44A0 Content-Type: text/plain; charset="koi8-r" Content-Transfer-Encoding: base64 z+7r/Ofu4uDy5ev8IFRpbXVyIFNhZmluIPXu8uXrIOH7IO7y7ufi4PL8IPHu7uH55e3o5SwgIltQ QVRDSCB2MiAyLzNdIHNxbDoNCnVwZGF0ZWQgZXhwbGljaXQgY29udmVyc2lvbiB0YWJsZSIuDQo= ------=_NextPart_000_13DD_01D76A21.F98F44A0 Content-Type: application/ms-tnef; name="winmail.dat" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="winmail.dat" eJ8+IjoVAQaQCAAEAAAAAAABAAEAAQeQBgAIAAAA4wQAAAAAAADnAAEIgAcAEwAAAElQTS5PdXRs b29rLlJlY2FsbACCBgEDkAYATAYAABoAAAADACYAAQAAAAIBMQABAAAAtwEAAFBDREZFQjA5AAEA AgDUAAAAAAAAADihuxAF5RAaobsIACsqVsIAAHBzdHByeC5kbGwAAAAAAAAAAOkv63WWUESGg7h9 5SKqSUgAAEMAOgBcAFUAcwBlAHIAcwBcAHQALgBzAGEAZgBpAG4AXABBAHAAcABEAGEAdABhAFwA TABvAGMAYQBsAFwATQBpAGMAcgBvAHMAbwBmAHQAXABPAHUAdABsAG8AbwBrAFwAdABzAGEAZgBp AG4AQAB0AGEAcgBhAG4AdABvAG8AbAAuAG8AcgBnACgAMgApAC4AbwBzAHQAAAAuAAAAAAAAAIui JllSNbhOoRw9UUW7nRMBAMO2jhD3dRHOtM0AqgC7tuYAAAAAAA4AAEYAAAAAAAAAi6ImWVI1uE6h HD1RRbudEwcAw7aOEPd1Ec60zQCqALu25gAAAAAADgAASgiCOojS9kWN4O2oPrAKRgAAAAAO3AAA AAAAABAAAADNRlOZqWtbQ6S7pOr8Jl61OgAAAFJFOiBbUEFUQ0ggdjIgMi8zXSBzcWw6IHVwZGF0 ZWQgZXhwbGljaXQgY29udmVyc2lvbiB0YWJsZQAAHgBwAAEAAAA/AAAAAQjO8uf74jogW1BBVENI IHYyIDIvM10gc3FsOiB1cGRhdGVkIGV4cGxpY2l0IGNvbnZlcnNpb24gdGFibGUAAAsAAQ4BAAAA AwAUDgEAAAAeACgOAQAAADMAAAAwMDAwMDAwNAF0c2FmaW5AdGFyYW50b29sLm9yZwF0c2FmaW5A dGFyYW50b29sLm9yZwAAHgApDgEAAAAzAAAAMDAwMDAwMDQBdHNhZmluQHRhcmFudG9vbC5vcmcB dHNhZmluQHRhcmFudG9vbC5vcmcAAB4A+j8BAAAAFQAAAHRzYWZpbkB0YXJhbnRvb2wub3JnAAAA AAIBAGgBAAAAEAAAAM1GU5mpa1tDpLuk6vwmXrUDAFKACCAGAAAAAADAAAAAAAAARgAAAAAahQAA AQAAAAsAHw4BAAAAAgH4DwEAAAAQAAAAkkHSx//B80Cg7Zk0EhJ3LAIB+g8BAAAAEAAAAIuiJllS NbhOoRw9UUW7nRMDAP4PBQAAAAIBCRABAAAAUQEAAE0BAABFAgAATFpGdQX4McoDAAoAcmNwZzEy NSIyA0N0ZXgFQmJp/mQEAAMwAQMB9wqAAqQD4wkCAGNoCsBzZXQw/iAHEwKAEHMAUARWCFUHsu8S RQ5RAwERRzIGAAbDEkXOMwRGEUkTX2Y0EN4B0N40EeYSUwjvCfc7G08OMHY1EkIMYGMAUAsJAWQz DjYR0AumAzBodG1sHwAhGIUMASA1H3EnY2ZvIdAJ4CIRDDAnEVAiETe/IhUOUCIgAUAioCOCNSJW biAHYQhwBgFmC4AhwWbvJEIiQSP4IcFlAFAioCbT/yYWIkIjAiOJJLEikSchIjO7IkInIjkkFQsw IiA4JBIALCAiW1BBVEMISCB2FWAyLzNdACBzcWw6IHVw5GRhDrBkIA7AC1AN4OZpBUAFoG52BJAA kAIgwiABkWxlIi4gJgKxLn0hOQqxCoB9MUAAAAADAA00/T+tDgMADzT9P60OAgEUNAEAAAAQAAAA 6S/rdZZQRIaDuH3lIqpJSAIB4mUBAAAAFAAAAMyLbzESI0ZFoQb333y6KVgABml9AgHjZQEAAAAV AAAAFMyLbzESI0ZFoQb333y6KVgABml9AAAAAgF/AAEAAACNAAAAMDAwMDAwMDA4QkEyMjY1OTUy MzVCODRFQTExQzNENTE0NUJCOUQxMzA3MDBDM0I2OEUxMEY3NzUxMUNFQjRDRDAwQUEwMEJCQjZF NjAwMDAwMDAwMDAwRDAwMDA0QTA4ODIzQTg4RDJGNjQ1OERFMEVEQTgzRUIwMEE0NjAwMDAwMDAw MjhERDAwMDAAAAAAAwAGEIMsXhsDAAcQXQAAAAMAEBAAAAAAAwAREAEAAAAeAAgQAQAAAF4AAADP 7uv85+7i4PLl6/xUSU1VUlNBRklO9e7y5evh++7y7ufi4PL88e7u4fnl7ejlLCJQQVRDSFYyMi8z U1FMOlVQREFURURFWFBMSUNJVENPTlZFUlNJT05UQUJMRSIAAABgqQ== ------=_NextPart_000_13DD_01D76A21.F98F44A0-- 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 10C256EC40; Mon, 28 Jun 2021 02:46:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 10C256EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1624837594; bh=u3yPhB/hSAdscqy8SDF0Pz3Ogw/3DkedDXfxvdiQ9FA=; 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=wAhB5/lbnpw9gy8ziVHXX6tXAwmpGrmV2nQGvGvKK3PmlR8SLqkH2kgVZpzTkM9rr TxCNzEniPrxI4bOqws5q1uk0Ju06Fzzyoey5AX/kKuDh+JRSJTz29Ftt+78ujZ61Sy lHMgJsoKCuzDxvJDSoZ9OPl6Y/xUncQOMd4mv4BU= Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (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 C1FAE6EC40 for ; Mon, 28 Jun 2021 02:46:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C1FAE6EC40 Received: by smtp44.i.mail.ru with esmtpa (envelope-from ) id 1lxeTv-0003mH-Ow; Mon, 28 Jun 2021 02:46:32 +0300 To: "'Mergen Imeev'" Cc: References: <39ac18a73adc891f9ec615491c512b1a32237ed8.1623396615.git.tsafin@tarantool.org> <3e0a06c9-5ac4-fe91-8125-f216bc0b9085@tarantool.org> In-Reply-To: <3e0a06c9-5ac4-fe91-8125-f216bc0b9085@tarantool.org> Date: Mon, 28 Jun 2021 02:46:27 +0300 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQH9YP+QCEY6e5VusJ0O6Ha7mUcGiQGPNGQgAQmg0A0CxayJlg== Content-Language: ru X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD954DFF1DC42D673FBE6C6848F3EB6EB89AE0756428E32ECE9182A05F538085040EFB2144E207613F850D3FFFCC9A66332FC7445C4DBCD105752CA2418CA97D64D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE73F300A97FDD4E158EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006373ABD2822AD894AA78638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D863BC1A51E381B3A4E4EF38F998EAFD7A117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC8C7ADC89C2F0B2A5A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18C26CFBAC0749D213D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE599709FD55CB46A6B2D370F7B14D4BC4D8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE365B78C30F681404DBA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7E1BCFB2C0BE3F189731C566533BA786AA5CC5B56E945C8DA X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2368A440D3B0F6089093C9A16E5BC824A2A04A2ABAA09D25379311020FFC8D4AD70E3F9139E691156DF134562F76BA63E X-C1DE0DAB: 0D63561A33F958A5C02B1F4928A82D2BD8FAAE022F4020EEC8D5305065812804D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA753C350047980234DB410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3454E98140CCACB5B337614EF32D6A497CB100D30301138E78C5F1EDF13FC52FB3A2700254C09AD5501D7E09C32AA3244C876A006D58152BCF26D24D26AE0AF75FE8FBBEFAE1C4874C729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojBC0PCXY5SAU9vTl2Xd2d8Q== X-Mailru-Sender: 6CA451E36783D721CBEA96CEA26D325DEA799DD91931182076AFA043B62A6898B7CBEF92542CD7C82F97C478340294DCC77752E0C033A69E0F0C7111264B8915FF1320A92A5534336C18EFA0BB12DBB0 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 2/3] sql: updated explicit conversion table 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" Message-ID: <20210627234627.Iz_ZPSXan7F0sz2_4sG5geodvrxIfG4lESYPce-Pl6k@z> : From: Mergen Imeev : Subject: Re: [PATCH v2 2/3] sql: updated explicit conversion table :=20 : Thank you for the fixes you did. See my 27 comments below. :=20 : 1. In general I do not think that it is good idea to squash all = previous : three : patches to one patch. I think that the first of previous patches was : quite good, : the second was useless and the third had some issues, but now all = these : patches : are squashed. The idea was to split modification to implicit and explicit changes.=20 And taking into account that changes in behavior should be at the same=20 commit with modified tests those patches grew a bit. If we split=20 code and test then commits are not passing build/test separately. :=20 : On Fri, Jun 11, 2021 at 10:48:12AM +0300, Timur Safin wrote: : > * fixes for `BOOLEAN` expressions in explicit converstion tables : > : > We need to modify explicit casts table according to the RFC = developed : > previously in /doc/rfc/5910-consistent-sql-lua-types.md. This patch : > introduces changes where BOOLEAN type involved, thus, for simplicity : > sake, we mark unchanged cells in the table below as '.' : 2. I still do not think that path to RFC is needed here.=20 It's matter of taste. : Also, what about '.'? They mark unchanged 'Y'/'S' cells (as I said before). Or I've not=20 understood your clearly.=20 : > * introduce `ANY` as target for explicit conversions : > : > For consistency sake it was decided to provide : > `CAST(anything TO ANY)` as allowed, but still noop. : > : 3. I still do not like that you added cast to unsupported field. That was part of RFC we all agreed upon, not my own invention. :=20 : 4. Why typeof() of value cast to any returns 'any'? : tarantool> box.execute('select typeof(cast(1 AS any));') : --- : - metadata: : - name: COLUMN_1 : type: string : rows: : - ['any'] : ... :=20 : tarantool> box.execute('select typeof(cast(1 AS scalar));') : --- : - metadata: : - name: COLUMN_1 : type: string : rows: : - ['integer'] : ... Good observation, thanks! `cast(1 as scalar)` should return expression=20 attributed as `scalar` also. (Because why this cast then? Values should be left untouched, only type of expression reattributed. :=20 : > * We had to rename %wildcard token to ANYTHING, since we needed : > token ANY for real life usage. : > : 5. Just a thought - why you cannot use 'ANYTHING' for field type ANY? : Or 'ANY_KW', how it was already done for INTEGER? As %wildcard token we should use separate, artificial lexem, not any=20 of actually used. :=20 : > * As a byproduct, we fixed #6010 to make cast to unsigned behaving : reasonably : > : 6. I think this change is good enough to have its own patch. It's still part of implicit casts fixes, and affect some(same) set of = tests. [i.e. those tests will be modified multiple times along a patches flow = in The same patchset] But yes, in this case there was separate ticket, with specific = test-case, Thus it's easy to extract. :=20 : > * To make sure that all consistency checks are systematic, we have : introduced : > special test for checking conversion rules - e_casts.test.lua. = This : > patch introduces explicit table part: : > : > * e_casts.test.lua is generating expressions of `CAST(input AS = output)` : > form. All combinations check whether we expectedly succeeded or : failed, : > given the explicit conversion table from RFC = 5910-consistent-sql-lua- : types.md; : > : > * At the moment we skip DECIMALs, ARRAYs and MAPs as not yet : > fully supported. Need to be revisited later; : > : > * NB! there is `-verbose` mode one may activate to enable more = detailed : > reporting during debugging. : > : 7. I used './test-run.py sql-tap/e_casts.test.lua --verbose -j1' and = saw : this: : [001] sql-tap/e_casts.test.lua memtx [001] TAP : version 13 : [001] 1..3 : [001] # e_casts - check consistency of implicit conversion table : [001] 1..169 : [001] ok - /home/mergen/work/dev/test/sql-tap/e_casts.test.lua:245 : [any,any] nil ~=3D nil ... : [001] ok - /home/mergen/work/dev/test/sql-tap/e_casts.test.lua:245 : [unsigned,number] 2 ~=3D 1 : [001] ok - /home/mergen/work/dev/test/sql-tap/e_casts.test.lua:245 : [unsigned,decimal] nil ~=3D nil : [001] ok - /home/mergen/work/dev/test/sql-tap/e_casts.test.lua:245 : [unsigned,uuid] nil ~=3D nil : [001] ok - /home/mergen/work/dev/test/sql-tap/e_casts.test.lua:245 : [unsigned,array] nil ~=3D nil : [001] ok - /home/mergen/work/dev/test/sql-tap/e_casts.test.lua:245 : [unsigned,map] nil ~=3D nil : [001] ok - /home/mergen/work/dev/test/sql-tap/e_casts.test.lua:245 : [unsigned,scalar] 2 ~=3D 1 : ... :=20 : What is it? Have you looked into code of this test? In this subtest we check that implicit conversion table is symmetric in their cells (i.e. if there is any sort of conversion from type A to B then there have to be some kind sort of reverse conversion from type B to A) Agreed though, that 2 !=3D 1, and especially nil ~=3D nil, look funny = and=20 Confusing without looking into sources, and I will provide more = human-readable output there. :=20 : > Relates to #5910, #6009 : > Part of #4470 : > Fixes #6010 : > : > +int : > +mem_to_uint(struct Mem *mem) : > +{ : > + assert(mem->type < MEM_TYPE_INVALID); : > + if (mem->type =3D=3D MEM_TYPE_UINT) : > + return 0; : > + if (mem->type =3D=3D MEM_TYPE_INT) { : > + mem_set_uint(mem, (uint64_t)mem->u.i); : 8. Could you explain to me, what it this? Since when -1 can be cast to : UNSIGNED? This is rather (usual) reinterpret_cast<> for conversion of=20 negative signed integer to unsigned.=20 : Where it is described? Why this is allowed? : tarantool> box.execute('select CAST(-1 as unsigned);') : --- : - metadata: : - name: COLUMN_1 : type: unsigned : rows: : - [18446744073709551615] : ... Now looking into RFC table for explicit casts I do see it's slightly extension of an agreed upon "conditional" conversion (i.e. if signed integer has positive value then it should be convertible, but raise error otherwise).=20 So yes, thanks for catch! :=20 :=20 : > + return 0; : > + } : > + if ((mem->type & (MEM_TYPE_STR | MEM_TYPE_BIN)) !=3D 0) : > + return bytes_to_uint(mem); : > + if (mem->type =3D=3D MEM_TYPE_DOUBLE) : > + return double_to_uint(mem); : > + return -1; : > +} : > + : > int : > mem_to_int(struct Mem *mem) : > { : > assert(mem->type < MEM_TYPE_INVALID); : > - if ((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) !=3D 0) : > + if (mem->type =3D=3D MEM_TYPE_INT) : > + return 0; : > + if (mem->type =3D=3D MEM_TYPE_UINT) { : > + mem_set_int(mem, (int64_t)mem->u.u, false); : 9. So, you want to set unsigned value as integer. Could you tell me = what : does : this change do? At least it will change type attribution (that's important because of different allowed ranges of values). : > @@ -1017,13 +989,8 @@ mem_cast_explicit(struct Mem *mem, enum = field_type : type) : > switch (mem->type) { : 10. Just a thought. I see no harm to have this switch here, but it has : only two : options. Why not change it to 'if'? Wanted to say that I kept switch to look consistent with code above and=20 below, but then realized that more complicated code with switch cases has already been refactored to functions and this switch left alone,=20 and only ifs lying around it.=20 So, yep, will switch switch to ifs. That would fit better to the = context. ... : > diff --git a/test/sql-tap/e_casts.test.lua = b/test/sql-tap/e_casts.test.lua : > new file mode 100755 : > index 000000000..32d7e8e0c : > --- /dev/null : > +++ b/test/sql-tap/e_casts.test.lua : > @@ -0,0 +1,340 @@ : > +#!/usr/bin/env tarantool : > +local tap =3D require("tap") : > +local test =3D tap.test("errno") : > + : > +test:plan(1) : > + : > +local yaml =3D require("yaml") : > +local ffi =3D require("ffi") : > + : > +local verbose =3D 0 : > + : > +if arg[1] =3D=3D '-v' or arg[1] =3D=3D '--verbose' then : > + verbose =3D 1 : > +end : > + : > +ffi.cdef [[ : > + enum field_type { : > + FIELD_TYPE_ANY =3D 0, : > + FIELD_TYPE_UNSIGNED, : > + FIELD_TYPE_STRING, : > + FIELD_TYPE_NUMBER, : > + FIELD_TYPE_DOUBLE, : > + FIELD_TYPE_INTEGER, : > + FIELD_TYPE_BOOLEAN, : > + FIELD_TYPE_VARBINARY, : > + FIELD_TYPE_SCALAR, : > + FIELD_TYPE_DECIMAL, : > + FIELD_TYPE_UUID, : > + FIELD_TYPE_ARRAY, : > + FIELD_TYPE_MAP, : > + field_type_MAX : > + }; : > +]] : 11. Since this cdef is only copy-paste from current actual enum, I = still see : not reason to keep it here. Or you could use one from module.h, if you = want : to keep it. Nope, we could not use module.h - it's been built much, much later in = the current build flow. Module_api and testing are currently independent = targets. and this unnecessary dependency looks redundant, copy-pastes are bad, = but sometimes they are excusable to avoid introduction of unnecessary = troubles (in already quite fragile build process) : > + : > +-- not all types implemented/enabled at the moment : > +-- but we do keep their projected status in the : > +-- spec table : > +local enabled_type =3D { : > + [t_any] =3D false, -- there is no way in SQL to = instantiate ANY : type expression : > + [t_unsigned] =3D true, : > + [t_string] =3D true, : > + [t_double] =3D true, : > + [t_integer] =3D true, : > + [t_boolean] =3D true, : > + [t_varbinary] =3D true, : > + [t_number] =3D true, : > + [t_decimal] =3D false, : > + [t_uuid] =3D true, : > + -- [t_date] =3D false, : > + -- [t_time] =3D false, : > + -- [t_timestamp]=3D false, : > + -- [t_interval] =3D False, : > + [t_array] =3D false, : > + [t_map] =3D false, : > + [t_scalar] =3D true, : > +} : > + : > +-- Enabled types which may be targets for explicit casts : > +local enabled_type_cast =3D table.deepcopy(enabled_type) : > +enabled_type_cast[t_any] =3D true : > + : > +-- table of _TSV_ (tab separated values) : > +-- copied from sql-lua-tables-v5.xls : > +local explicit_casts_table_spec =3D { : > + [t_any] =3D {"Y", "S", "Y", "S", "S", "S", "S", "S", "S", = "S", "S", : "S", "S"}, : > + [t_unsigned]=3D {"Y", "Y", "Y", "Y", "Y", "" , "" , "Y", "Y", = "" , "" , : "" , "Y"}, : > + [t_string] =3D {"Y", "S", "Y", "S", "S", "S", "Y", "S", "S", = "S", "S", : "S", "Y"}, : > + [t_double] =3D {"Y", "S", "Y", "Y", "S", "" , "" , "Y", "Y", = "" , "" , : "" , "Y"}, : > + [t_integer] =3D {"Y", "S", "Y", "Y", "Y", "" , "" , "Y", "Y", = "" , "" , : "" , "Y"}, : > + [t_boolean] =3D {"Y", "" , "Y", "" , "" , "Y", "" , "" , "" , = "" , "" , : "" , "Y"}, : > + [t_varbinary]=3D{"Y", "" , "Y", "N", "" , "" , "Y", "" , "" , = "S", "" , : "" , "Y"}, : > + [t_number] =3D {"Y", "S", "Y", "Y", "S", "" , "" , "Y", "Y", = "" , "" , : "" , "Y"}, : > + [t_decimal] =3D {"Y", "S", "Y", "S", "S", "" , "" , "Y", "Y", = "" , "" , : "" , "Y"}, : > + [t_uuid] =3D {"Y", "" , "Y", "" , "" , "" , "Y", "" , "" , = "Y", "" , : "" , "Y"}, : > + [t_array] =3D {"Y", "N", "Y", "N", "N", "N", "" , "" , "" , = "" , "Y", : "" , "N"}, : > + [t_map] =3D {"Y", "N", "Y", "N", "N", "N", "" , "" , "" , = "" , "" , : "Y", "N"}, : > + [t_scalar] =3D {"Y", "S", "Y", "S", "S", "S", "S", "S", "S", = "S", "" , : "" , "Y"}, : > +} : 12. Again, why there is 'N' and '' both in table? Could you show me = where in : RFC we have this table that contains both '' and 'N'? This is also explained with - because of RFC! ;) Kinda. In this case = that's the way it looked in frozen materials (excel) used for RFC generation which I've = published=20 as attachments to = https://github.com/tarantool/tarantool/wiki/SQL-Lua-Consistent-Type-Speci= fication wiki page. OTOH ' ' and 'N' are surely interchangeable, and could be = easily massaged=20 in the editor, so I'll change them to always be ' ' (to make it less = confusing for others). ... : > + : > + for _, from in ipairs(proper_order) do : > + local line =3D '' : > + for _, to in ipairs(proper_order) do : > + line =3D line .. string.format("%2s |", : human_cast(table[from][to])) : > + end : > + line =3D string.sub(line, 1, #line-1) : > + local s =3D string.format("%2d.%10s |%s|", from, = type_names[from], : line) : > + print(s) : > + end : > + print(string.format("%"..max_len.."s%s", "", banner)) : > +end : 13. Since you use '-verbose' mode only in your debug, can you move = these : functions out from here? I still do not see any use of this code. Out from here to where? I don't get it. I do print implicit and explicit = tables content if ran with '--verbose' option. And this is great = debugging helper while you modify those tables. Have you ever run this test outside of test-run infrastructure? Like=20 ``` =E2=9C=94 ~/tarantool/test 18:57 $ cat ~/bin/sql-tap-run.sh=20 #/usr/bin/bash root=3D$(git rev-parse --show-toplevel) rm -f *.snap *.xlog LUA_PATH=3D"$root/test/sql-tap/lua/?.lua;$root/test/sql/lua/?.lua;" \ $prefix $root/build/src/tarantool $* =E2=9C=94 ~/tarantool/test 18:57 $ ~/bin/sql-tap-run.sh sql-tap/e_casts.test.lua --verbose |& head = -40 TAP version 13 1..3 | 0 | 1 | 2 | 4 | 5 | 6 | 7 | 3 | 9 |10 |11 |12 | 8 | +---+---+---+---+---+---+---+---+---+---+---+---+---+ 0. any | | | | | | | | | | | | | | 1. unsigned | Y | Y | Y | Y | Y | | | Y | | | | | Y | 2. string | Y | S | Y | S | S | S | Y | S | | S | | | Y | 4. double | Y | S | Y | Y | S | | | Y | | | | | Y | 5. integer | Y | S | Y | Y | Y | | | Y | | | | | Y | 6. boolean | Y | | Y | | | Y | | | | | | | Y | 7. varbinary | Y | | Y | | | | Y | | | S | | | Y | 3. number | Y | S | Y | Y | S | | | Y | | | | | Y | 9. decimal | | | | | | | | | | | | | | 10. uuid | Y | | Y | | | | Y | | | Y | | | Y | 11. array | | | | | | | | | | | | | | 12. map | | | | | | | | | | | | | | 8. scalar | Y | S | Y | S | S | S | S | S | | S | | | Y | +---+---+---+---+---+---+---+---+---+---+---+---+---+ | 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 | | Y | S | | S | | | Y | 4. double | | S | Y | Y | S | | | Y | | | | | Y | 5. integer | | S | Y | Y | Y | | | Y | | | | | Y | 6. boolean | | | | | | Y | | | | | | | Y | 7. varbinary | | | Y | | | | Y | | | S | | | Y | 3. number | | S | Y | Y | S | | | Y | | | | | Y | 9. decimal | | | | | | | | | | | | | | 10. uuid | | | Y | | | | Y | | | Y | | | Y | 11. array | | | | | | | | | | | | | | 12. map | | | | | | | | | | | | | | 8. scalar | | S | S | S | S | S | S | S | | S | | | S | +---+---+---+---+---+---+---+---+---+---+---+---+---+ # e_casts - check consistency of implicit conversion table 1..169 ok - sql-tap/e_casts.test.lua:245 [any,any] nil ~=3D nil ok - sql-tap/e_casts.test.lua:245 [any,unsigned] nil ~=3D nil ok - sql-tap/e_casts.test.lua:245 [any,string] nil ~=3D nil ok - sql-tap/e_casts.test.lua:245 [any,double] nil ~=3D nil ``` Ain't them pretty? ;) : > + : > +local gen_type_samples =3D { : > + [t_unsigned] =3D {"0", "1", "2"}, : 14. I believe you once said something about testing values that are = close to : all limits of numeric types. Why I cannot see them here? This is very good point - I'll add some boundary values. : > +local function catch_query(query) : > + local result =3D {pcall(box.execute, query)} : > + : > + if not result[1] or result[3] ~=3D nil then : > + return false, result[3] : > + end : > + return true, result[2] : > +end : 15. Since you use all these functions and do not want to remove them, = I : suggest : to add comments to them. Yeah, these are much easier to use than those we have in sqltester.lua.=20 Will provide some comments. :=20 : > + : > +-- 1. Check explicit casts table : > +local function test_check_explicit_casts(test) : > + -- checking validity of all `CAST(from AS to)` combinations : > + test:plan(322) : > + for _, from in ipairs(proper_order) do : > + for _, to in ipairs(proper_order) do : > + -- skip ANY, DECIMAL, UUID, etc. : 16. Why UUID is skipped? Also, why ANY is skipped?=20 No, in this test we do not skip uuids, they already been integrated into test. That's rather stale/outdated comment. Sorry for the = confusion. : You can get values of type : ANY by using CAST(values AS ANY), I believe. Or I am wrong? Yes and no. Given expression of a form `CAST(x as ANY)` we do have=20 expression attributed with type ANY, but there is still no direct=20 way to write literal of type ANY. Having said that we indeed could use more complicated expressions, not only literals. So yes, we may use such expressions. [But today I'm not actually certain we will not disable these kinds=20 of casts entirely, despite them agreed in the RFC. Along with all possible changes many want to introduce for scalars. So let see=20 where we will go in the nearest future. Sigh. ] :=20 : > + if enabled_type[from] and enabled_type_cast[to] then : > + local gen =3D gen_explicit_cast_from_to(from, to) : > + local failures =3D {} : > + local successes =3D {} : > + local castable =3D false : > + local expected =3D explicit_casts[from][to] : > + : > + if verbose > 0 then : > + print(expected, yaml.encode(gen)) : > + end : > + : > + : > + -- ok, we aggregated stats for c_maybe mode - check = it : now : > + if expected =3D=3D c_maybe then : > + local title =3D string.format("%s =3D> = %s", : > + #gen and : gen[1]..'...' or '', : > + = human_cast(expected)) : > + test:ok(castable, label_for(from, to, = title), : > + failures) : > + end : 16. I already said my expectations about these tests. Since you know : (and print) : all values used in the test, why cannot you determine if the cast = should : fail? : For example, this test is actually useless if `CAST(1.0 AS UNSIGNED)` : fails, since : this cast is marked as 'sometimes'. If you do so, you can just print = 'Y' : or 'N'. : Also, why your tests does not check unallowed casts? Or, at least, = they : are not : shown. It's actually testing entire table (with only exception of unsupported=20 today types). You see the line? local expected =3D explicit_casts[from][to] and then if expected =3D=3D c_yes then test:ok(true =3D=3D ok, label_for(from, to, title)) elseif expected =3D=3D c_no then test:ok(false =3D=3D ok, label_for(from, to, title)) else The problem is in TAP reporting - I did not find any way to have passing test but displaying a number of expected failures. They all look = identical=20 now (as passes), and all looks as castable, even if they are not. Will modify label_for() as promised above to clearly indicate that some of them expected to fail. : > --- a/test/sql-tap/in1.test.lua : > +++ b/test/sql-tap/in1.test.lua : > @@ -100,7 +100,7 @@ test:do_execsql_test( : > test:do_execsql_test( : > "in-1.7", : > [[ : > - SELECT a+ 100*CAST((a BETWEEN 1 and 3) AS INTEGER) FROM t1 = ORDER : BY b : > + SELECT a+ 100*(CASE WHEN (a BETWEEN 1 and 3) THEN 1 ELSE 0 = END) : FROM t1 ORDER BY b : 17. Since you already change this line, add spaces before and after = all : operations with two operands. Hmm, ok :=20 : > ]], { : > -- : > 101, 102, 103, 4, 5, 6, 7, 8, 9, 10 : > @@ -157,7 +157,7 @@ test:do_execsql_test( : > test:do_execsql_test( : > "in-2.5", : > [[ : > - SELECT a+100*(CAST(b IN (8,16,24) AS INTEGER)) FROM t1 = ORDER BY b : > + SELECT a+100*(CASE WHEN b IN (8,16,24) THEN 1 ELSE 0 END) = FROM t1 : ORDER BY b : 18. Same. Ok :=20 : > ]], { : > -- : > 1, 2, 103, 104, 5, 6, 7, 8, 9, 10 : > @@ -207,7 +207,7 @@ test:do_execsql_test( : > test:do_execsql_test( : > "in-2.10", : > [[ : > - SELECT a FROM t1 WHERE LEAST(0, CAST(b IN (a,30) AS INT)) = <> 0 : > + SELECT a FROM t1 WHERE LEAST(0, CASE WHEN (b IN (a,30)) = THEN 1 : ELSE 0 END) <> 0 : 19. I think a space should be after ',' in '(a,30)'. Ok : > diff --git a/test/sql-tap/in4.test.lua b/test/sql-tap/in4.test.lua : > index 8442944b9..588daefec 100755 : > --- a/test/sql-tap/in4.test.lua : > +++ b/test/sql-tap/in4.test.lua : > @@ -133,7 +133,7 @@ test:do_execsql_test( : > SELECT b FROM t2 WHERE a IN (1.0, 2.1) : > ]], { : > -- : > - "one" : > + "one", "two" : 21. Why the result changed? Good question! You see I've added case when implicit cast from double to = integer worked expectedly and brought us 2 rows, but here, by some = reason=20 implicit cast from 2.1 didn't match integer 2. I need to investigate reasons, at the moment I do not recollect why it was ok for me. :=20 : > -- : > }) : > : > diff --git a/test/sql-tap/misc3.test.lua = b/test/sql-tap/misc3.test.lua : > index 313484b5d..c2dc67355 100755 : > --- a/test/sql-tap/misc3.test.lua : > +++ b/test/sql-tap/misc3.test.lua : > @@ -510,7 +510,7 @@ test:do_execsql_test( : > test:do_execsql_test( : > "misc-8.2", : > [[ : > - SELECT count(*) FROM t3 WHERE 1+CAST((b IN ('abc','xyz')) = AS : INTEGER)=3D=3D2 : > + SELECT count(*) FROM t3 WHERE b IN ('abc','xyz') : 22. What does this test checks now and what it checked before? That was actually a regression test for SQLite bug #668 ``` -- Ticket #668: VDBE stack overflow occurs when the left-hand side -- of an IN expression is NULL and the result is used as an integer, = not -- as a jump. ``` It makes no sense today for our conditions, and I believe it should=20 be kept here just for not reducing coverage. [Though I'd delete=20 subtests for #668 if we would care about logical system in our tests. = :)] : > diff --git a/test/sql/boolean.result b/test/sql/boolean.result : > index 177a39fb9..b268eb2fe 100644 : > --- a/test/sql/boolean.result : > +++ b/test/sql/boolean.result : > @@ -502,23 +502,13 @@ INSERT INTO t3 VALUES (4, false) : > -- Check CAST from BOOLEAN to the other types. : > SELECT cast(true AS INTEGER), cast(false AS INTEGER); : > | --- : > - | - metadata: : > - | - name: COLUMN_1 : > - | type: integer : > - | - name: COLUMN_2 : > - | type: integer : > - | rows: : > - | - [1, 0] : > + | - null : > + | - 'Type mismatch: can not convert TRUE to integer' : 23. Since all these CAST are now fails, I think it is better to divide = this : query into three. Otherwise only the first cast is checked. Into 3? Have you actually meant 2 in this case? :=20 : > | ... : > SELECT cast(true AS NUMBER), cast(false AS NUMBER); : > | --- : > - | - metadata: : > - | - name: COLUMN_1 : > - | type: number : > - | - name: COLUMN_2 : > - | type: number : > - | rows: : > - | - [1, 0] : > + | - null : > + | - 'Type mismatch: can not convert TRUE to number' : 24. Same. The same confusion - we will receive 2 separate test, why 3? : > @@ -545,25 +535,13 @@ SELECT cast(true AS BOOLEAN), cast(false AS : BOOLEAN); : > -- Check CAST to BOOLEAN from the other types. : > SELECT cast(100 AS BOOLEAN), cast(1 AS BOOLEAN), cast(0 AS = BOOLEAN); : > | --- : > - | - metadata: : > - | - name: COLUMN_1 : > - | type: boolean : > - | - name: COLUMN_2 : > - | type: boolean : > - | - name: COLUMN_3 : > - | type: boolean : > - | rows: : > - | - [true, true, false] : > + | - null : > + | - 'Type mismatch: can not convert 100 to boolean' : > | ... : 25. Same. Division of this select into 3 separate selects makes no sense at all -=20 they will receive the same result for the same condition, the only=20 reasonable case now would be: ``` - SELECT cast(100 AS BOOLEAN), cast(1 AS BOOLEAN), cast(0 AS BOOLEAN); + SELECT cast(100 AS BOOLEAN); ``` :=20 : > SELECT cast(0.123 AS BOOLEAN), cast(0.0 AS BOOLEAN); : > | --- : > - | - metadata: : > - | - name: COLUMN_1 : > - | type: boolean : > - | - name: COLUMN_2 : > - | type: boolean : > - | rows: : > - | - [true, false] : > + | - null : > + | - 'Type mismatch: can not convert 0.123 to boolean' : > | ... : 26. Same. Similarly - only 1 expression is necessary : > diff --git a/test/sql/types.result b/test/sql/types.result : > index 687ca3b15..90a8bc5ec 100644 : > --- a/test/sql/types.result : > +++ b/test/sql/types.result : > @@ -1084,8 +1081,11 @@ box.execute("SELECT CAST(123 AS UNSIGNED);") : > ... : > box.execute("SELECT CAST(-123 AS UNSIGNED);") : > --- : > -- null : > -- 'Type mismatch: can not convert -123 to unsigned' : > +- metadata: : > + - name: COLUMN_1 : > + type: unsigned : > + rows: : > + - [18446744073709551493] : > ... : 27. Where does such behaviour is described? Ok, as a said above such reinterpret_cast<> was rather an extension of RFC, and will return back conditional failure. Thanks, Timur