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 ED7E06EC40; Thu, 3 Jun 2021 00:05:30 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org ED7E06EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1622667931; bh=Yb6JQu5oq/DY5ro+bBGneCyLQzxcKJL5gDWKykIhD34=; 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=kybti9F+s7XpxUxtVab5YYkNlaBKKZqG880SHaouGJWhblYSvNbd1q4B3EHQzk+WT Qd/a0QX5RkiViKQeo2VKbkgr06TBvmMFbxZpeqe9x9sGVGHWACKJfYKhDdH9bGCcjZ ZUVG+cBkDYLG4IPy6Wg5r09WFcpwhruV2fVE2V4Y= Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (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 D98DF6EC40 for ; Thu, 3 Jun 2021 00:04:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D98DF6EC40 Received: by smtp48.i.mail.ru with esmtpa (envelope-from ) id 1loY2Z-0007My-Uf; Thu, 03 Jun 2021 00:04:40 +0300 To: "'Mergen Imeev'" Cc: References: In-Reply-To: Date: Thu, 3 Jun 2021 00:04:39 +0300 Message-ID: <053301d757f2$e6f07760$b4d16620$@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: AQIN143y2CAuVXbaZAstAFvLluhQBgEPlDb9AwABhb+qc3ar8A== Content-Language: ru X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D5B0DA836B685C544BBC2A69B1B4100B389BF69B7A224D7C182A05F538085040F6D885A462F8944F9E0EF4C6EC72E58ADF6AD36DAF094AF01A53CB476860642A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE70C5E0F71D77D667BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637A975286280E7A0BD8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D884B32381746EE5FAE74C370A65BC9BF5117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18C26CFBAC0749D213D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE41BF15D38FB6CB3AA68A47777D5C6D9CD8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE34CB6874B0BCFF0B86136E347CC761E07C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637BBEA499411984DA1EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2368A440D3B0F6089093C9A16E5BC824A2A04A2ABAA09D25379311020FFC8D4AD494FFA51BD827F93591579B4FFB5EE4C X-C1DE0DAB: 0D63561A33F958A5A8F7962AE108D9568CFD02B891F51E34FEB8ADB79463BF20D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75FBC5FED0552DA851410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34B7CBFF60649FF266B9697D94E0A1E5749942AAF05B958B1E1DADEA19A543D74B53A8077BA41EDAFA1D7E09C32AA3244C59850126A33FE7CD60FF177E0C0325D51DD47778AE04E04D729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj+mfSpkNmA2rtMZ5JxTgB0w== X-Mailru-Sender: B5B6A6EBBD94DAD840208BF9E14C1DD267CAAAC99014A0C176CFC172EC47B02239F8EBC7FD6E94111EC9E4A2C82A33BC8C24925A86E657CE0C70AEE3C9A96FBAB3D7EE8ED63280BE112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 3/3] sql: introduced explicit casts test e_casts.test.lua 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 3/3] sql: introduced explicit casts test : e_casts.test.lua :=20 : Thank you for the patch. See 13 comments below. :=20 : On Tue, May 25, 2021 at 12:01:02PM +0300, Timur Safin wrote: : > * e_casts.test.lua is generating expressions of `cast(input as = output)` : > form. And checks whether we expectedly succeed or fail, given the : > previously agreed excplicit conversion table from : > /doc/rfc/5910-consistent-sql-lua-types.md; : > : > * At the moment we skip DECIMALs, UUIDs, ARRAYs and MAPs as not yet : > fully supported. Need to be reenabled later; : > : > * there is `-verbose` mode one may activate to enable more detailed : > reporting. : 1. How to use this mode? I mean, : ./test-run.py sql-tap/e_casts.test.lua --verbose : gives a lot simpler result that I expect from this much of code. For debugging of various failures I run it directly via script as simple = as that: 20:05 $ ~/bin/sql-tap-run.sh sql-tap/e_casts.test.lua -v | 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 | 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 | +---+---+---+---+---+---+---+---+---+---+---+---+---+ | 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 | | | | | 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 | | | | | | Y | 3. number | | S | Y | Y | S | | | Y | | | | | Y | 9. decimal | | | | | | | | | | | | | | 10. uuid | | | | | | | | | | | | | | 11. array | | | | | | | | | | | | | | 12. map | | | | | | | | | | | | | | 8. scalar | | 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:244 [any,any] nil ~=3D nil ok - sql-tap/e_casts.test.lua:244 [any,unsigned] nil ~=3D nil ok - sql-tap/e_casts.test.lua:244 [any,string] nil ~=3D nil ok - sql-tap/e_casts.test.lua:244 [any,double] nil ~=3D nil ... I need to see those tables (explicit and implicit) visualized to make=20 sure I have them correctly entered and be compatible with written to = RFC. This output is incompatible (and unnecessary) for TAP mode, that's why = it's=20 not activated by default. :=20 : 2. What means e_casts? If it is for explicit casts, why not name it : something : like "explicit_cast.test.lua". Because it's not planned to be only about explicit casts :) [You'll see it's development in the next version of patchset] And this strange e_* prefix is inherited from the times when I wanted to resurrect e_expr.test.lua Now, having asked about it, I do realize it makes no much sense and probably should be removed. Will rename to sql-tap/casts.test.lua :=20 : 3. As far as I remember, "sometimes" doesn't mean that CAST(1.0 as = UNSIGNED) : can return 1 or can throw an error. It means that there are rules that : define : when values of one type can be converted to values of another type. = So, : instead of checking failures for "maybe" we can check that "from" = value : should fail or not according to these rules and use proper test. Yes, sometimes meant that some part of data might be safely converted, = but others will not. And here we should prepare proper set where some values are valid for conversion and others are not. (See my answer below) :=20 : Also, using this approach we can try to use randomized "from" values. = Just : thought. I thought about it some day, but no, randomized data will introduce = unnecessary volatility to testing and will not add any extra value to this = particular case. A well prepared data-set provides here much better results, when we actually check only compile time reactions, and not run-time. Though, eventually this approach might be modified for fuzzing of this = part of code, and there randomization will be necessary, but not today. :=20 : 4. Why you test only literals? Nothing indicates that values from = spaces : shouldn't be tested here. At the moment of writing this patchset I've not yet figured out how to conveniently deal with generated fields of expected types. Now, for = implicit conversion type I do have easy approach, and will add this case for = run-time explicit conversion of different type expressions. :=20 : > : > Relates to #5910, #6009 : > Part of #4407 : 5. Wrong issue number. Also, I still think that "relates to" is not : needed here. Yup thanks for the catch about 4470! [But I'd still keep relation to consistent-type spec - that's part of larger work] :=20 : > --- : > test/sql-tap/e_casts.test.lua | 355 = ++++++++++++++++++++++++++++++++++ : > 1 file changed, 355 insertions(+) : > create mode 100755 test/sql-tap/e_casts.test.lua : > : > diff --git a/test/sql-tap/e_casts.test.lua = b/test/sql-tap/e_casts.test.lua : > new file mode 100755 : > index 000000000..eafebd5bc : > --- /dev/null : > +++ b/test/sql-tap/e_casts.test.lua : > @@ -0,0 +1,355 @@ : > +#!/usr/bin/env tarantool : > +local test =3D require("sqltester") : 6. What point to use sqltester here is you do not use its functions? = You : can use : 'tap' instead. Good point - sqltester is not used here. :=20 : > +test:plan(222) : > + : > +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 : 7. How to use '-v' option? :=20 : > + 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 : > + }; : > +]] : 8. Is there any point to use ffi here? Can't you use something = simpler? : For example: : local types =3D { : any =3D 0, : unsigned =3D 1, : ... :=20 : Than instead of t_any you can use types.any and so on. Originally the point was to just copy-paste the definition from = Tarantool code as-is, without any massaging or modifications. [Ideally it should be extracted from the actual code src/box/field_def.h = code.=20 And I'll probably do so, eventually, because field_def.h is prepared for = such extraction, and this place is marked with /** \cond public */=20 /** \endcond public */ boundaries, so enum could be easily extracted] :=20 : If you start from 1 instead of 0 you can also use type_names instead = of : proper_order. At the moment I don't consider that getting rid of ffi.cdef will = simplify anything or provide any extra value.=20 :=20 : > + : > +-- Date/time/interval types to be uncommented and used : > +-- once corresponding box implementation completed : > +local t_any =3D ffi.C.FIELD_TYPE_ANY : > +local t_unsigned =3D ffi.C.FIELD_TYPE_UNSIGNED : > +local t_string =3D ffi.C.FIELD_TYPE_STRING : > +local t_number =3D ffi.C.FIELD_TYPE_NUMBER : > +local t_double =3D ffi.C.FIELD_TYPE_DOUBLE : > +local t_integer =3D ffi.C.FIELD_TYPE_INTEGER : > +local t_boolean =3D ffi.C.FIELD_TYPE_BOOLEAN : > +local t_varbinary =3D ffi.C.FIELD_TYPE_VARBINARY : > +local t_scalar =3D ffi.C.FIELD_TYPE_SCALAR : > +local t_decimal =3D ffi.C.FIELD_TYPE_DECIMAL : > +-- local t_date =3D -1 : > +-- local t_time =3D -2 : > +-- local t_timestamp =3D -3 : > +-- local t_interval =3D -4 : > +local t_uuid =3D ffi.C.FIELD_TYPE_UUID : > +local t_array =3D ffi.C.FIELD_TYPE_ARRAY : > +local t_map =3D ffi.C.FIELD_TYPE_MAP : > + : > +local proper_order =3D { : > + t_any, : > + t_unsigned, : > + t_string, : > + t_double, : > + t_integer, : > + t_boolean, : > + t_varbinary, : > + t_number, : > + t_decimal, : > + t_uuid, : > + -- t_date, : > + -- t_time, : > + -- t_timestamp, : > + -- t_interval, : 9. I think it is better to check them for "wrong type name" or remove = these : types. I don't quite get what is "check for 'wrong type name'"? Could you = please elaborate? (Those unused type will be added in the nearest times, in any case) :=20 : > + t_array, : > + t_map, : > + t_scalar, : > +} : > + : > +local type_names =3D { : > + [t_any] =3D 'any', : > + [t_unsigned] =3D 'unsigned', : > + [t_string] =3D 'string', : > + [t_double] =3D 'double', : > + [t_integer] =3D 'integer', : > + [t_boolean] =3D 'boolean', : > + [t_varbinary] =3D 'varbinary', : > + [t_number] =3D 'number', : > + [t_decimal] =3D 'decimal', : > + [t_uuid] =3D 'uuid', : > + -- [t_date] =3D 'date', : > + -- [t_time] =3D 'time', : > + -- [t_timestamp] =3D 'timestamp', : > + -- [t_interval] =3D 'interval', : > + [t_array] =3D 'array', : > + [t_map] =3D 'map', : > + [t_scalar] =3D 'scalar', : > +} : > + : > +-- 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 false, : > + -- [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, : > +} : > + : > +-- table of _TSV_ (tab separated values) : > +-- copied from sql-lua-tables-v5.xls // TNT implicit today : > +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"}, : > +} : 10. Why there is 'N' and '' both in table? Can't be make it use only = one : of these? Because this is actual copy-paste from original xls on which RFC was = based on. :) [See attachments in = https://github.com/tsafin/tarantool/wiki/SQL-Lua-Consistent-Type-Specific= ation] :=20 : > + : > +-- local extra_checks =3D false : > +local explicit_casts =3D {} : > +-- local implicit_casts =3D {} : > + : > +-- implicit conversion table is considered consistent if : > +-- it's sort of symmetric against diagonal : > +-- (not necessary that always/sometimes are matching : > +-- but at least something should be presented) : > + : > +--[[ local function check_table_consistency(table) : > + for _, i in ipairs(proper_order) do : > + local string =3D '' : > + for _, j in ipairs(proper_order) do : > + print(i, j) : > + -- local item =3D implicit_casts[i][j] : > + -- string =3D string .. (xlat[item] or ' ') : > + end : > + print(string) : > + end : > +end : > +]] : > + : > + -- if there is enabled extra checks then check ocnsistency of = input : tables : > + -- just to make sure their sanity : > +--[[ if extra_checks then : > + check_table_consistency() : > + end : > + ]] : > + : 11. Please drop commented part of code. This is cleaned up in the current version, including implicit casts = support,=20 see the next version of patchset. :=20 : > +local c_no =3D 0 : > +local c_maybe =3D 1 : > +local c_yes =3D 2 : > + : > +local function normalize_cast(v) : > + local xlat =3D { : > + ['Y'] =3D c_yes, : > + ['S'] =3D c_maybe, : > + ['N'] =3D c_no, : > + } : > + return xlat[v ~=3D nil and v or 'N'] : > +end : > + : > +local function human_cast(v) : > + local xlat =3D { : > + [c_yes] =3D 'Y', : > + [c_maybe] =3D 'S', : > + [c_no] =3D ' ' : > + } : > + return xlat[v ~=3D nil and v or c_no] : > +end : > + : > +local function load_casts_spec(spec_table) : > + local casts =3D {} : > + for i, t_from in ipairs(proper_order) do : > + local row =3D spec_table[t_from] : > + casts[t_from] =3D {} : > + for j, t_to in ipairs(proper_order) do : > + if enabled_type[t_from] and enabled_type[t_to] then : > + casts[t_from][t_to] =3D : normalize_cast(spec_table[t_from][j]) : > + end : > + end : > + end : > + -- if there is enabled extra checks then check ocnsistency of = input : tables : > + -- just to make sure their sanity : > +--[[ if extra_checks then : > + check_table_consistency() : > + end ]] : > + : > + return casts : > +end : > + : > +explicit_casts =3D load_casts_spec(explicit_casts_table_spec) : > + : > +if verbose > 0 then : > + local function show_casts_table(table) : 12. I believe word 'table' is reserved for in Lua. Not sure that it is = good : idea to overwrite this reserved word. Apparently it's not reserved and we could reuse it as identifier in local context. But thanks for reminding - that would be very confusing=20 the moment I'd need to use table.insert or something. (Strange that = luacheck which is always activated in my vscode editor did not complain here a = lot) :=20 : > + local max_len =3D #"12. varbinary" + 1 : > + : > + -- show banner : > + local col_names =3D '' : > + for i, t_val in ipairs(proper_order) do : > + col_names =3D col_names .. string.format("%2d |", = t_val) : > + end : > + col_names =3D string.sub(col_names, 1, #col_names-1) : > + print(string.format("%"..max_len.."s|%s|", "", col_names)) : > + -- show splitter : > + local banner =3D = '+---+---+---+---+---+---+---+---+---+---+---+--- : +---+' : > + print(string.format("%"..max_len.."s%s", "", banner)) : > + : > + for i, from in ipairs(proper_order) do : > + local line =3D '' : > + for j, 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 : > + : > + show_casts_table(explicit_casts) : > +end : > + : > +local function merge_tables(...) : > + local n =3D select('#', ...) : > + local tables =3D {...} : > + local result =3D {} : > + : > + for i=3D1,n do : > + local t =3D tables[i] : > + --print(yaml.encode(t)) : > + assert(type(tables[i]) =3D=3D 'table') : > + for j,v in pairs(t) do : > + table.insert(result, v) : > + end : > + end : > + return result : > +end : > + : > +local gen_type_samples =3D { : > + [t_unsigned] =3D {"0", "1", "2"}, : > + [t_integer] =3D {"-678", "-1", "0", "1", "2", "3", = "678"}, : > + [t_double] =3D {"0.0", "123.4", "-567.8"}, : > + [t_string] =3D {"''", "'1'", "'abc'", "'def'", "'TRUE'", : "'FALSE'"}, : > + [t_boolean] =3D {"false", "true", "null"}, : > + [t_varbinary] =3D {"X'312E3233'", "X'2D392E3837'", : "X'302E30303031'"}, : > +} : > + : > +local function gen_type_exprs(type) : > + if type =3D=3D t_number then : > + return merge_tables(gen_type_samples[t_unsigned], : > + gen_type_samples[t_integer], : > + gen_type_samples[t_double]) : > + end : > + if type =3D=3D t_scalar then : > + return merge_tables(gen_type_samples[t_unsigned], : > + gen_type_samples[t_integer], : > + gen_type_samples[t_double], : > + gen_type_samples[t_string], : > + gen_type_samples[t_boolean], : > + gen_type_samples[t_varbinary]) : > + end : > + return gen_type_samples[type] or {} : > +end : 13. Do you need this function? You can just expand gen_type_samples : using its : code. Function allows to avoid copy-paste for values from meta types such as Number and scalar.=20 :=20 : This comment can be implemented for some other functions, for example : the next : one. I think it will make code simpler to understand. [If I correctly understood your sentence] Nope, copy paste will make=20 code much more verbose, and less observable. :=20 : > + : > +local function gen_sql_cast_from_to(t_from, t_to) : > + local queries =3D {} : > + local from_exprs =3D gen_type_exprs(t_from) : > + local to_typename =3D type_names[t_to] : > + for _, expr in pairs(from_exprs) do : > + local query =3D string.format([[ select cast(%s as %s); ]], = expr, : to_typename) : > + table.insert(queries, query) : > + end : > + return queries : > +end : > + : > +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 : > + : > +local function label_for(from, to, query) : > + local parent_frame =3D debug.getinfo(2, "nSl") : > + local filename =3D parent_frame.source:sub(1,1) =3D=3D "@" and : parent_frame.source:sub(2) : > + local line =3D parent_frame.currentline : > + return string.format("%s+%d:[%s,%s] %s", filename, line, : > + type_names[from], type_names[to], query) : > +end : > + : > +for i, from in ipairs(proper_order) do : > + for j, to in ipairs(proper_order) do : > + -- skip ANY, DECIMAL, UUID, etc. : > + if enabled_type[from] and enabled_type[to] then : > + local cell =3D explicit_casts[from][to] : > + local gen =3D gen_sql_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 : > + for i, v in pairs(gen) do : > + local ok, result : > + ok, result =3D catch_query(v) : > + if verbose > 0 then : > + print(string.format("ok =3D %s, result =3D %s, = query =3D : %s", : > + ok, result, v)) : > + : > + end : > + -- print(v, 'ok'..yaml.encode(ok), : 'result'..yaml.encode(result)) : > + if expected =3D=3D c_yes then : > + test:ok(true =3D=3D ok, label_for(from, to, v)) : > + elseif expected =3D=3D c_no then : > + test:ok(false =3D=3D ok, label_for(from, to, = v)) : > + else : > + -- we can't report immediately for c_maybe because = some : > + -- cases allowed to fail, so postpone decision : > + if ok then : > + castable =3D true : > + table.insert(successes, {result, v}) : > + else : > + table.insert(failures, {result, v}) : > + end : > + end : > + end : > + : > + -- ok, we aggregated stats for c_maybe mode - check it = now : > + if expected =3D=3D c_maybe then : > + test:ok(castable, label_for(from, to, #gen and = gen[1] : or ''), : > + failures) : > + end : > + end : > + end : > +end : > + : > + : > +test:finish_test() : > -- : > 2.29.2 : >