From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Timur Safin <tsafin@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 3/3] sql: updated implicit conversion table
Date: Sun, 20 Jun 2021 21:52:56 +0300 [thread overview]
Message-ID: <4b33bd96-174f-4749-a749-f07247d05a7a@tarantool.org> (raw)
In-Reply-To: <f055ac47b7d961b1946774da97e3457699268fef.1623396615.git.tsafin@tarantool.org>
Thank you for the patch. See 33 comments below.
On Fri, Jun 11, 2021 at 10:48:13AM +0300, Timur Safin wrote:
> * Changed implicit casts table according to consistent types RFC
>
> * fixed following directions for implicit conversions
> - string to double
> - double to integer
> - varbinary to string
>
> * got rid of mem_cast_implicit_old() as unnecessary, use always
> the updated mem_cast_implicit()
>
> * in addition to update of all relevant tests there is extended
> e_casts.test.lua used for checking of implicit conversion rules:
>
> * there is table information sanity check - e_casts.test.lua checks
> that implicit table content is sane, i.e. it's sort of symmetric
> for all defined combinations [from, to]
>
> * for the test we use inserts in specially crafted table as checks
> availability of implicit conversions between type values
> * Temporary TCASTS table used with sequence primary key
> * All fields of a form `{name = "unsigned", type = "unsigned", is_nullable = true}`
> used for all enabled types
1. Since you said that you fix 'implicit cast', not just 'implicit cast for
insert', please fix all implicit casts. Also, your changes added
implicit cast
for comparison in some cases, even though it should be removed according
to RFC.
>
> Relates to #5910, #6009
> Closes #4470
>
> * Additionally we have fixed unexpected results for sum of implicitly
> converted numbers
2. I believe this change should be moved to its own patch.
>
> Fixing unexpected results after implicit conversion to
> the signed or unsigned integer from well-formed string.
>
> ```
> tarantool> box.execute([[select '1' + '2';]])
> ---
> - metadata:
> - name: COLUMN_1
> type: scalar
> rows:
> - [3]
> ...
>
> tarantool> box.execute([[select '1' + '-2';]])
> ---
> - metadata:
> - name: COLUMN_1
> type: scalar
> rows:
> - [18446744073709551615]
> ...
>
> tarantool> box.execute([[select '-1' + '-2';]])
> ---
> - null
> - 'Failed to execute SQL statement: integer is overflowed'
> ...
> ```
3. So, this is how it works now? I do not see any description.
>
> Fixes #5756
> ---
> src/box/sql/mem.c | 107 ++++-----------------
> src/box/sql/mem.h | 6 --
> src/box/sql/util.c | 3 +-
> src/box/sql/vdbe.c | 8 +-
> src/box/sql/vdbeaux.c | 2 +-
> test/sql-tap/e_casts.test.lua | 136 ++++++++++++++++++++++++++-
> test/sql-tap/in4.test.lua | 17 +++-
> test/sql-tap/numcast.test.lua | 2 +-
> test/sql-tap/tkt-9a8b09f8e6.test.lua | 40 ++++----
> test/sql/types.result | 104 ++++++++++----------
> 10 files changed, 248 insertions(+), 177 deletions(-)
>
> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index 8bb6b672c..d6b114a81 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -1033,95 +1033,24 @@ mem_cast_implicit(struct Mem *mem, enum field_type type)
> }
> switch (type) {
> case FIELD_TYPE_UNSIGNED:
> - if (mem->type == MEM_TYPE_UINT)
> - return 0;
> - if (mem->type == MEM_TYPE_DOUBLE)
> - return double_to_uint(mem);
> - return -1;
> - case FIELD_TYPE_STRING:
> - if (mem->type == MEM_TYPE_STR)
> - return 0;
> - if (mem->type == MEM_TYPE_UUID)
> - return uuid_to_str0(mem);
> - return -1;
> - case FIELD_TYPE_DOUBLE:
> - if (mem->type == MEM_TYPE_DOUBLE)
> - return 0;
> - if ((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) != 0)
> - return int_to_double(mem);
> - return -1;
> - case FIELD_TYPE_INTEGER:
> if ((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) != 0)
> return 0;
4. I tried to insert negative integer and I got this:
tarantool> box.execute('create table t (u unsigned primary key);')
---
- row_count: 1
...
tarantool> box.execute('insert into t values (-1);')
---
- null
- 'Tuple field 1 (U) type does not match one required by operation:
expected unsigned,
got integer'
...
Is it right?
> - if (mem->type == MEM_TYPE_DOUBLE)
> - return double_to_int(mem);
> - return -1;
> - case FIELD_TYPE_BOOLEAN:
> - if (mem->type == MEM_TYPE_BOOL)
> - return 0;
> - return -1;
> - case FIELD_TYPE_VARBINARY:
> - if ((mem->type & (MEM_TYPE_BIN | MEM_TYPE_MAP |
> - MEM_TYPE_ARRAY)) != 0)
> - return 0;
> - if (mem->type == MEM_TYPE_UUID)
> - return uuid_to_bin(mem);
> - return -1;
> - case FIELD_TYPE_NUMBER:
> - if (mem_is_num(mem))
> - return 0;
> - return -1;
> - case FIELD_TYPE_MAP:
> - if (mem->type == MEM_TYPE_MAP)
> - return 0;
> - return -1;
> - case FIELD_TYPE_ARRAY:
> - if (mem->type == MEM_TYPE_ARRAY)
> - return 0;
> - return -1;
> - case FIELD_TYPE_SCALAR:
> - if ((mem->type & (MEM_TYPE_MAP | MEM_TYPE_ARRAY)) != 0)
> - return -1;
> - return 0;
> - case FIELD_TYPE_UUID:
> - if (mem->type == MEM_TYPE_UUID)
> - return 0;
> - if (mem->type == MEM_TYPE_STR)
> - return str_to_uuid(mem);
> - if (mem->type == MEM_TYPE_BIN)
> - return bin_to_uuid(mem);
> - return -1;
> - case FIELD_TYPE_ANY:
> - return 0;
> - default:
> - break;
> - }
> - return -1;
> -}
> -
> -int
> -mem_cast_implicit_old(struct Mem *mem, enum field_type type)
> -{
> - if (mem->type == MEM_TYPE_NULL)
> - return 0;
> - switch (type) {
> - case FIELD_TYPE_UNSIGNED:
> - if (mem->type == MEM_TYPE_UINT)
> - return 0;
> if (mem->type == MEM_TYPE_DOUBLE)
> return double_to_uint_precise(mem);
> if (mem->type == MEM_TYPE_STR)
> return bytes_to_uint(mem);
> return -1;
> case FIELD_TYPE_STRING:
> - if ((mem->type & (MEM_TYPE_STR | MEM_TYPE_BIN)) != 0)
> + if (mem->type == MEM_TYPE_STR)
> return 0;
> + if (mem->type == MEM_TYPE_UUID)
> + return uuid_to_str0(mem);
> if ((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) != 0)
> return int_to_str0(mem);
> if (mem->type == MEM_TYPE_DOUBLE)
> return double_to_str0(mem);
> - if (mem->type == MEM_TYPE_UUID)
> - return uuid_to_str0(mem);
5. Why you moved these lines?
> + if (mem->type == MEM_TYPE_BIN)
> + return bin_to_str(mem);
> return -1;
> case FIELD_TYPE_DOUBLE:
> if (mem->type == MEM_TYPE_DOUBLE)
> @@ -1129,25 +1058,28 @@ mem_cast_implicit_old(struct Mem *mem, enum field_type type)
> if ((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) != 0)
> return int_to_double(mem);
> if (mem->type == MEM_TYPE_STR)
> - return bin_to_str(mem);
> + return bytes_to_double(mem);
> return -1;
> case FIELD_TYPE_INTEGER:
> if ((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) != 0)
> return 0;
> - if (mem->type == MEM_TYPE_STR)
> - return bytes_to_int(mem);
6. Same.
> if (mem->type == MEM_TYPE_DOUBLE)
> return double_to_int_precise(mem);
> + if (mem->type == MEM_TYPE_STR)
> + return bytes_to_int(mem);
> return -1;
> case FIELD_TYPE_BOOLEAN:
> if (mem->type == MEM_TYPE_BOOL)
> return 0;
> return -1;
> case FIELD_TYPE_VARBINARY:
> - if (mem->type == MEM_TYPE_BIN)
> + if ((mem->type & (MEM_TYPE_BIN | MEM_TYPE_MAP |
> + MEM_TYPE_ARRAY)) != 0)
7. What about this? Where it is described?
> return 0;
> if (mem->type == MEM_TYPE_UUID)
> return uuid_to_bin(mem);
> + if (mem->type == MEM_TYPE_STR)
> + return str_to_bin(mem);
> return -1;
> case FIELD_TYPE_NUMBER:
> if (mem_is_num(mem))
> @@ -1156,11 +1088,11 @@ mem_cast_implicit_old(struct Mem *mem, enum field_type type)
> return mem_to_number(mem);
> return -1;
> case FIELD_TYPE_MAP:
> - if (mem->type == MEM_TYPE_MAP)
> + if (mem_is_map(mem))
8. What is the point of this change? Please revert it.
> return 0;
> return -1;
> case FIELD_TYPE_ARRAY:
> - if (mem->type == MEM_TYPE_ARRAY)
> + if (mem_is_array(mem))
9. Same.
> return 0;
> return -1;
> case FIELD_TYPE_SCALAR:
> @@ -1175,6 +1107,8 @@ mem_cast_implicit_old(struct Mem *mem, enum field_type type)
> if (mem->type == MEM_TYPE_BIN)
> return bin_to_uuid(mem);
> return -1;
> + case FIELD_TYPE_ANY:
> + return 0;
> default:
> break;
> }
> @@ -1460,7 +1394,7 @@ get_number(const struct Mem *mem, struct sql_num *number)
> if (mem->type == MEM_TYPE_INT) {
> number->i = mem->u.i;
> number->type = MEM_TYPE_INT;
> - number->is_neg = true;
> + number->is_neg = mem->u.i < 0;
10. Why? All values of type MEM_TYPE_INT are negative by default.
> return 0;
> }
> if (mem->type == MEM_TYPE_UINT) {
> @@ -1473,13 +1407,6 @@ get_number(const struct Mem *mem, struct sql_num *number)
> return -1;
> if (sql_atoi64(mem->z, &number->i, &number->is_neg, mem->n) == 0) {
> number->type = number->is_neg ? MEM_TYPE_INT : MEM_TYPE_UINT;
> - /*
> - * The next line should be removed along with the is_neg field
> - * of struct sql_num. The integer type tells us about the sign.
> - * However, if it is removed, the behavior of arithmetic
> - * operations will change.
> - */
> - number->is_neg = false;
11. Why you did not removed 'is_neg' field from 'struct sql_number'?
What is the
point of having it there?
> return 0;
> }
> if (sqlAtoF(mem->z, &number->d, mem->n) != 0) {
> diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
> index b3cd5c545..706cad43c 100644
> --- a/src/box/sql/mem.h
> +++ b/src/box/sql/mem.h
> @@ -753,12 +753,6 @@ mem_cast_explicit(struct Mem *mem, enum field_type type);
> int
> mem_cast_implicit(struct Mem *mem, enum field_type type);
>
> -/**
> - * Convert the given MEM to given type according to legacy implicit cast rules.
> - */
> -int
> -mem_cast_implicit_old(struct Mem *mem, enum field_type type);
> -
> /**
> * Return value for MEM of INTEGER type. For MEM of all other types convert
> * value of the MEM to INTEGER if possible and return converted value. Original
> diff --git a/src/box/sql/util.c b/src/box/sql/util.c
> index c556b9815..69b1a3937 100644
> --- a/src/box/sql/util.c
> +++ b/src/box/sql/util.c
> @@ -958,9 +958,8 @@ sql_add_int(int64_t lhs, bool is_lhs_neg, int64_t rhs, bool is_rhs_neg,
> *res = lhs + rhs;
> return 0;
> }
> - *is_res_neg = is_rhs_neg ? (uint64_t)(-rhs) > (uint64_t) lhs :
> - (uint64_t)(-lhs) > (uint64_t) rhs;
> *res = lhs + rhs;
> + *is_res_neg = *res < 0;
12. Is this right?
tarantool> box.execute('select 18446744073709551606+-1;')
---
- metadata:
- name: COLUMN_1
type: integer
rows:
- [-11]
...
tarantool> box.execute('select -1+18446744073709551606;')
---
- metadata:
- name: COLUMN_1
type: integer
rows:
- [-11]
...
Before this change:
tarantool> box.execute('select -1+18446744073709551606;')
---
- metadata:
- name: COLUMN_1
type: integer
rows:
- [18446744073709551605]
...
tarantool> box.execute('select 18446744073709551606+-1;')
---
- metadata:
- name: COLUMN_1
type: integer
rows:
- [18446744073709551605]
...
> return 0;
> }
>
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index 32d02d96e..0dc28e2d6 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -1660,7 +1660,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
> } else if (type == FIELD_TYPE_STRING) {
> if (mem_cmp_str(pIn3, pIn1, &res, pOp->p4.pColl) != 0) {
> const char *str =
> - mem_cast_implicit_old(pIn3, type) != 0 ?
> + mem_cast_implicit(pIn3, type) != 0 ?
13. Is this right?
tarantool> box.execute('create table t2(s string primary key);')
---
- row_count: 1
...
tarantool> box.execute([[insert into t2 values('0');]])
---
- row_count: 1
...
tarantool> box.execute('select * from t2 where s < 1;')
---
- metadata:
- name: S
type: string
rows:
- ['0']
...
> mem_str(pIn3) : mem_str(pIn1);
> diag_set(ClientError, ER_SQL_TYPE_MISMATCH, str,
> "string");
> @@ -1671,7 +1671,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
> type = FIELD_TYPE_NUMBER;
> if (mem_cmp_num(pIn3, pIn1, &res) != 0) {
> const char *str =
> - mem_cast_implicit_old(pIn3, type) != 0 ?
> + mem_cast_implicit(pIn3, type) != 0 ?
14. Is this right?
tarantool> box.execute('create table t3(n number primary key);')
---
- row_count: 1
...
tarantool> box.execute([[insert into t3 values(0);]])
---
- row_count: 1
...
tarantool> box.execute([[select * from t3 where n < '1';]])
---
- metadata:
- name: N
type: number
rows:
- [0]
...
> mem_str(pIn3) : mem_str(pIn1);
> diag_set(ClientError, ER_SQL_TYPE_MISMATCH, str,
> "numeric");
> @@ -1682,7 +1682,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */
> assert(mem_is_str(pIn3) && mem_is_same_type(pIn3, pIn1));
> if (mem_cmp_str(pIn3, pIn1, &res, pOp->p4.pColl) != 0) {
> const char *str =
> - mem_cast_implicit_old(pIn3, type) != 0 ?
> + mem_cast_implicit(pIn3, type) != 0 ?
15. What changes after this?
> mem_str(pIn3) : mem_str(pIn1);
> diag_set(ClientError, ER_SQL_TYPE_MISMATCH, str,
> "string");
> @@ -2218,7 +2218,7 @@ case OP_MakeRecord: {
> if (types != NULL) {
> pRec = pData0;
> do {
> - mem_cast_implicit_old(pRec++, *(types++));
> + mem_cast_implicit(pRec++, *(types++));
16. Same, what changes?
> } while(types[0] != field_type_MAX);
> }
>
> diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
> index 4a1fdb637..c8e242c2f 100644
> --- a/src/box/sql/vdbeaux.c
> +++ b/src/box/sql/vdbeaux.c
> @@ -2332,7 +2332,7 @@ sqlVdbeGetBoundValue(Vdbe * v, int iVar, u8 aff)
> sql_value *pRet = sqlValueNew(v->db);
> if (pRet) {
> mem_copy(pRet, pMem);
> - mem_cast_implicit_old(pRet, aff);
> + mem_cast_implicit(pRet, aff);
17. Same, what changes?
> }
> return pRet;
> }
> diff --git a/test/sql-tap/e_casts.test.lua b/test/sql-tap/e_casts.test.lua
> index 32d7e8e0c..952572921 100755
> --- a/test/sql-tap/e_casts.test.lua
> +++ b/test/sql-tap/e_casts.test.lua
> @@ -2,7 +2,7 @@
> local tap = require("tap")
> local test = tap.test("errno")
>
> -test:plan(1)
> +test:plan(3)
>
> local yaml = require("yaml")
> local ffi = require("ffi")
> @@ -137,6 +137,22 @@ local explicit_casts_table_spec = {
> [t_scalar] = {"Y", "S", "Y", "S", "S", "S", "S", "S", "S", "S", "" , "" , "Y"},
> }
>
> +local implicit_casts_table_spec = {
> + [t_any] = {"Y", "S", "S", "S", "S", "S", "S", "S", "S", "S", "S", "S", "S"},
> + [t_unsigned]= {"Y", "Y", "Y", "Y", "Y", "" , "" , "Y", "Y", "" , "" , "" , "Y"},
> + [t_string] = {"Y", "S", "Y", "S", "S", "" , "Y", "S", "S", "S", "" , "" , "Y"},
> + [t_double] = {"Y", "S", "Y", "Y", "S", "" , "" , "Y", "Y", "" , "" , "" , "Y"},
> + [t_integer] = {"Y", "S", "Y", "Y", "Y", "" , "" , "Y", "Y", "" , "" , "" , "Y"},
> + [t_boolean] = {"Y", "" , "" , "" , "" , "Y", "" , "" , "" , "" , "" , "" , "Y"},
> + [t_varbinary]={"Y", "" , "Y", "" , "" , "" , "Y", "" , "" , "S", "" , "" , "Y"},
> + [t_number] = {"Y", "S", "Y", "Y", "S", "" , "" , "Y", "Y", "" , "" , "" , "Y"},
> + [t_decimal] = {"Y", "S", "Y", "S", "S", "" , "" , "Y", "Y", "" , "" , "" , "Y"},
> + [t_uuid] = {"Y", "" , "Y", "" , "" , "" , "Y", "" , "" , "Y", "" , "" , "Y"},
> + [t_array] = {"Y", "" , "" , "" , "" , "" , "" , "" , "" , "" , "Y", "" , "" },
> + [t_map] = {"Y", "" , "" , "" , "" , "" , "" , "" , "" , "" , "" , "Y", "" },
> + [t_scalar] = {"Y", "S", "S", "S", "S", "S", "S", "S", "S", "S", "" , "" , "S"},
> +}
> +
> local c_no = 0
> local c_maybe = 1
> local c_yes = 2
> @@ -207,9 +223,31 @@ local function show_casts_table(table)
> end
>
> local explicit_casts = load_casts_spec(explicit_casts_table_spec, enabled_type, enabled_type_cast)
> +local implicit_casts = load_casts_spec(implicit_casts_table_spec, enabled_type, enabled_type)
>
> if verbose > 0 then
> show_casts_table(explicit_casts)
> + show_casts_table(implicit_casts)
> +end
> +
> +-- 0. check consistency of input conversion table
> +
> +-- 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 test_check_table_consistency(test)
> + test:plan(169)
> + for _, from in ipairs(proper_order) do
> + for _, to in ipairs(proper_order) do
> + test:ok((normalize_cast(implicit_casts[from][to]) ~= c_no) ==
> + (normalize_cast(implicit_casts[to][from]) ~= c_no),
> + label_for(from, to,
> + string.format("%s ~= %s",
> + implicit_casts[from][to],
> + implicit_casts[to][from])))
> + end
> + end
> end
>
> local function merge_tables(...)
> @@ -334,7 +372,103 @@ local function test_check_explicit_casts(test)
> end
> end
>
> +local table_name = 'TCASTS'
> +
> +local function _created_formatted_space(name)
> + local space = box.schema.space.create(name)
> + space:create_index('pk', {sequence = true})
> + local format = {{name = 'ID', type = 'unsigned', is_nullable = false}}
> + for _, type_id in ipairs(proper_order) do
> + if enabled_type[type_id] then
> + local type_name = type_names[type_id]
> + table.insert(format, {name = type_name, type = type_name, is_nullable = true})
> + end
> + end
> + if #format > 0 then
> + space:format(format)
> + end
> + return space
> +end
> +
> +local function _cleanup_space(space)
> + space:drop()
> +end
> +
> +-- implicit
> +local function gen_implicit_insert_from_to(table_name, from, to)
> + local queries = {}
> + local from_exprs = gen_type_exprs(from)
> + for _, from_e in pairs(from_exprs) do
> + table.insert(queries,
> + string.format([[ insert into %s("%s") values(%s); ]],
> + table_name, type_names[to], from_e))
> + end
> + return queries
> +end
> +
> +
> +-- 2. Check implicit casts table
> +local function test_check_implicit_casts(test)
> + test:plan(186)
> + local space = _created_formatted_space(table_name)
> + -- checking validity of all `from binop to` combinations
> + for _, from in ipairs(proper_order) do
> + for _, to in ipairs(proper_order) do
> + -- skip ANY, DECIMAL, UUID, etc.
> + if enabled_type[from] and enabled_type[to] then
> + local gen = gen_implicit_insert_from_to(table_name, from, to)
> + local failures = {}
> + local successes = {}
> + local castable = false
> + local expected = implicit_casts[from][to]
> +
> + if verbose > 0 then
> + print(expected, yaml.encode(gen))
> + end
> +
> + for _, v in pairs(gen) do
> + local ok, result
> + ok, result = catch_query(v)
> +
> + if verbose > 0 then
> + print(string.format("V> ok = %s, result = %s, query = %s",
> + ok, result, v))
> + end
> +
> + local title = string.format("%s => %s", v, human_cast(expected))
> + if expected == c_yes then
> + test:ok(true == ok, label_for(from, to, title))
> + elseif expected == c_no then
> + test:ok(false == ok, label_for(from, to, title))
> + else
> + -- we can't report immediately for c_maybe because some
> + -- cases allowed to fail, so postpone decision
> + if ok then
> + castable = 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 == c_maybe then
> + local title = string.format("%s => %s",
> + #gen and gen[1]..'...' or '',
> + human_cast(expected))
> + test:ok(castable, label_for(from, to, title),
> + failures)
> + end
> + end
> + end
> + end
> + _cleanup_space(space)
> +end
> +
> +test:test("e_casts - check consistency of implicit conversion table", test_check_table_consistency)
> test:test("e_casts - check explicit casts", test_check_explicit_casts)
> +test:test("e_casts - check implicit casts", test_check_implicit_casts)
18. Same comments as in previous letter.
>
> test:check()
> os.exit()
> diff --git a/test/sql-tap/in4.test.lua b/test/sql-tap/in4.test.lua
> index 588daefec..6aeaff5d5 100755
> --- a/test/sql-tap/in4.test.lua
> +++ b/test/sql-tap/in4.test.lua
> @@ -1,6 +1,6 @@
> #!/usr/bin/env tarantool
> local test = require("sqltester")
> -test:plan(52)
> +test:plan(53)
>
> --!./tcltestrunner.lua
> -- 2008 September 1
> @@ -128,15 +128,26 @@ test:do_execsql_test(
> })
>
> test:do_execsql_test(
> - "in4-2.6",
> + "in4-2.6.0",
> [[
> - SELECT b FROM t2 WHERE a IN (1.0, 2.1)
> + SELECT b FROM t2 WHERE a IN (1.0, 2.0)
> ]], {
> -- <in4-2.6>
> "one", "two"
> -- </in4-2.6>
> })
>
> +-- FIXME - IN [2.1] should convert to expected type of a
> +test:do_execsql_test(
> + "in4-2.6.1",
> + [[
> + SELECT b FROM t2 WHERE a IN (1.0, 2.1)
> + ]], {
> + -- <in4-2.6.1>
> + "one"
> + -- </in4-2.6.1>
> + })
> +
19. In the previous patch you changed the result and now you changed the
test.
Why?
> test:do_execsql_test(
> "in4-2.7",
> [[
> diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
> index 6ca1316d5..798b2dc77 100755
> --- a/test/sql-tap/numcast.test.lua
> +++ b/test/sql-tap/numcast.test.lua
> @@ -139,7 +139,7 @@ test:do_catchsql_test(
> test:do_execsql_test(
> "cast-2.9",
> [[
> - INSERT INTO t VALUES(2.1);
> + INSERT INTO t VALUES(2.0);
> SELECT * FROM t;
> ]], {
> 2, 9223372036854775808ULL, 18000000000000000000ULL
> diff --git a/test/sql-tap/tkt-9a8b09f8e6.test.lua b/test/sql-tap/tkt-9a8b09f8e6.test.lua
> index 67d6a1ccd..8ecf9f914 100755
> --- a/test/sql-tap/tkt-9a8b09f8e6.test.lua
> +++ b/test/sql-tap/tkt-9a8b09f8e6.test.lua
> @@ -239,23 +239,23 @@ test:do_execsql_test(
> -- </4.2>
> })
>
> -test:do_catchsql_test(
> +test:do_execsql_test(
> 4.3,
> [[
> SELECT x FROM t3 WHERE x IN ('1');
> ]], {
> -- <4.3>
> - 1, "Type mismatch: can not convert 1 to number"
> + 1.0
20. Why this does not throw an error now? Where this behaviour is described?
> -- </4.3>
> })
>
> -test:do_catchsql_test(
> +test:do_execsql_test(
> 4.4,
> [[
> SELECT x FROM t3 WHERE x IN ('1.0');
> ]], {
> -- <4.4>
> - 1, "Type mismatch: can not convert 1.0 to number"
> + 1.0
21. Same.
> -- </4.4>
> })
>
> @@ -279,23 +279,23 @@ test:do_execsql_test(
> -- </4.6>
> })
>
> -test:do_catchsql_test(
> +test:do_execsql_test(
> 4.7,
> [[
> SELECT x FROM t3 WHERE '1' IN (x);
> ]], {
> -- <4.7>
> - 1, "Type mismatch: can not convert 1 to number"
> + 1.0
22. Same.
> -- </4.7>
> })
>
> -test:do_catchsql_test(
> +test:do_execsql_test(
> 4.8,
> [[
> SELECT x FROM t3 WHERE '1.0' IN (x);
> ]], {
> -- <4.8>
> - 1, "Type mismatch: can not convert 1.0 to number"
> + 1.0
23. Same.
> -- </4.8>
> })
>
> @@ -319,23 +319,23 @@ test:do_execsql_test(
> -- </5.2>
> })
>
> -test:do_catchsql_test(
> +test:do_execsql_test(
> 5.3,
> [[
> SELECT x FROM t4 WHERE x IN ('1');
> ]], {
> -- <5.3>
> - 1, "Type mismatch: can not convert 1 to number"
> +
24. Same.
> -- </5.3>
> })
>
> -test:do_catchsql_test(
> +test:do_execsql_test(
> 5.4,
> [[
> SELECT x FROM t4 WHERE x IN ('1.0');
> ]], {
> -- <5.4>
> - 1, "Type mismatch: can not convert 1.0 to number"
> +
25. Same.
> -- </5.4>
> })
>
> @@ -349,13 +349,13 @@ test:do_execsql_test(
> -- </5.5>
> })
>
> -test:do_catchsql_test(
> +test:do_execsql_test(
> 5.6,
> [[
> SELECT x FROM t4 WHERE x IN ('1.11');
> ]], {
> -- <5.6>
> - 1, "Type mismatch: can not convert 1.11 to number"
> + 1.11
26. Same.
> -- </5.6>
> })
>
> @@ -379,23 +379,23 @@ test:do_execsql_test(
> -- </5.8>
> })
>
> -test:do_catchsql_test(
> +test:do_execsql_test(
> 5.9,
> [[
> SELECT x FROM t4 WHERE '1' IN (x);
> ]], {
> -- <5.9>
> - 1, "Type mismatch: can not convert 1 to number"
> +
27. Same.
> -- </5.9>
> })
>
> -test:do_catchsql_test(
> +test:do_execsql_test(
> 5.10,
> [[
> SELECT x FROM t4 WHERE '1.0' IN (x);
> ]], {
> -- <5.10>
> - 1, "Type mismatch: can not convert 1.0 to number"
> +
28. Same.
> -- </5.10>
> })
>
> @@ -409,13 +409,13 @@ test:do_execsql_test(
> -- </5.11>
> })
>
> -test:do_catchsql_test(
> +test:do_execsql_test(
> 5.12,
> [[
> SELECT x FROM t4 WHERE '1.11' IN (x);
> ]], {
> -- <5.12>
> - 1, "Type mismatch: can not convert 1.11 to number"
> + 1.11
29. Same.
> -- </5.12>
> })
>
> diff --git a/test/sql/types.result b/test/sql/types.result
> index 90a8bc5ec..20dbd2073 100644
> --- a/test/sql/types.result
> +++ b/test/sql/types.result
> @@ -1059,7 +1059,8 @@ box.execute("INSERT INTO t1 VALUES (0), (1), (2);")
> box.execute("INSERT INTO t1 VALUES (-3);")
> ---
> - null
> -- 'Type mismatch: can not convert -3 to unsigned'
> +- 'Tuple field 1 (ID) type does not match one required by operation: expected unsigned,
> + got integer'
30. Why the error changed?
> ...
> box.execute("SELECT id FROM t1;")
> ---
> @@ -1224,12 +1225,13 @@ box.execute("INSERT INTO t VALUES(1, true);")
> ...
> box.execute("INSERT INTO t VALUES(1, 'asd');")
> ---
> -- null
> -- 'Type mismatch: can not convert asd to varbinary'
> +- row_count: 1
> ...
> box.execute("INSERT INTO t VALUES(1, x'616263');")
> ---
> -- row_count: 1
> +- null
> +- Duplicate key exists in unique index "pk_unnamed_T_1" in space "T" with old tuple
> + - [1, "asd"] and new tuple - [1, "abc"]
31. Can you avoid this error? For example by changing 1 to 2. This would
allow
to avoid some changes below.
> ...
> box.execute("SELECT * FROM t WHERE v = 1")
> ---
> @@ -1253,8 +1255,7 @@ box.execute("SELECT * FROM t WHERE v = x'616263'")
> type: integer
> - name: V
> type: varbinary
> - rows:
> - - [1, 'abc']
> + rows: []
> ...
> box.execute("SELECT sum(v) FROM t;")
> ---
> @@ -1277,7 +1278,7 @@ box.execute("SELECT min(v) FROM t;")
> - name: COLUMN_1
> type: scalar
> rows:
> - - ['abc']
> + - ['asd']
> ...
> box.execute("SELECT max(v) FROM t;")
> ---
> @@ -1285,7 +1286,7 @@ box.execute("SELECT max(v) FROM t;")
> - name: COLUMN_1
> type: scalar
> rows:
> - - ['abc']
> + - ['asd']
> ...
> box.execute("SELECT count(v) FROM t;")
> ---
> @@ -1301,7 +1302,7 @@ box.execute("SELECT group_concat(v) FROM t;")
> - name: COLUMN_1
> type: string
> rows:
> - - ['abc']
> + - ['asd']
> ...
> box.execute("SELECT lower(v) FROM t;")
> ---
> @@ -1332,7 +1333,7 @@ box.execute("SELECT quote(v) FROM t;")
> - name: COLUMN_1
> type: string
> rows:
> - - ['X''616263''']
> + - ['X''617364''']
> ...
> box.execute("SELECT LEAST(v, x'') FROM t;")
> ---
> @@ -1351,8 +1352,7 @@ box.execute("SELECT v FROM t WHERE v = x'616263';")
> - metadata:
> - name: V
> type: varbinary
> - rows:
> - - ['abc']
> + rows: []
> ...
> box.execute("SELECT v FROM t ORDER BY v;")
> ---
> @@ -1360,11 +1360,11 @@ box.execute("SELECT v FROM t ORDER BY v;")
> - name: V
> type: varbinary
> rows:
> - - ['abc']
> + - ['asd']
> ...
> box.execute("UPDATE t SET v = x'636261' WHERE v = x'616263';")
> ---
> -- row_count: 1
> +- row_count: 0
> ...
> box.execute("SELECT v FROM t;")
> ---
> @@ -1372,7 +1372,7 @@ box.execute("SELECT v FROM t;")
> - name: V
> type: varbinary
> rows:
> - - ['cba']
> + - ['asd']
> ...
> box.execute("CREATE TABLE parent (id INT PRIMARY KEY, a VARBINARY UNIQUE);")
> ---
> @@ -2206,8 +2206,9 @@ box.execute([[INSERT INTO ti(i) VALUES (true);]])
> ...
> box.execute([[INSERT INTO ti(i) VALUES ('33');]])
> ---
> -- null
> -- 'Type mismatch: can not convert 33 to integer'
> +- autoincrement_ids:
> + - 4
> + row_count: 1
> ...
> box.execute([[INSERT INTO ti(i) VALUES (X'3434');]])
> ---
> @@ -2225,6 +2226,7 @@ box.execute([[SELECT * FROM ti;]])
> - [1, null]
> - [2, 11]
> - [3, 33]
> + - [4, 33]
> ...
> box.execute([[INSERT INTO td(d) VALUES (NULL);]])
> ---
> @@ -2256,8 +2258,9 @@ box.execute([[INSERT INTO td(d) VALUES (true);]])
> ...
> box.execute([[INSERT INTO td(d) VALUES ('33');]])
> ---
> -- null
> -- 'Type mismatch: can not convert 33 to double'
> +- autoincrement_ids:
> + - 4
> + row_count: 1
> ...
> box.execute([[INSERT INTO td(d) VALUES (X'3434');]])
> ---
> @@ -2275,6 +2278,7 @@ box.execute([[SELECT * FROM td;]])
> - [1, null]
> - [2, 11]
> - [3, 22.2]
> + - [4, 33]
> ...
> box.execute([[INSERT INTO tb(b) VALUES (NULL);]])
> ---
> @@ -2327,13 +2331,15 @@ box.execute([[INSERT INTO tt(t) VALUES (NULL);]])
> ...
> box.execute([[INSERT INTO tt(t) VALUES (11);]])
> ---
> -- null
> -- 'Type mismatch: can not convert 11 to string'
> +- autoincrement_ids:
> + - 2
> + row_count: 1
> ...
> box.execute([[INSERT INTO tt(t) VALUES (22.2);]])
> ---
> -- null
> -- 'Type mismatch: can not convert 22.2 to string'
> +- autoincrement_ids:
> + - 3
> + row_count: 1
> ...
> box.execute([[INSERT INTO tt(t) VALUES (true);]])
> ---
> @@ -2343,13 +2349,14 @@ box.execute([[INSERT INTO tt(t) VALUES (true);]])
> box.execute([[INSERT INTO tt(t) VALUES ('33');]])
> ---
> - autoincrement_ids:
> - - 2
> + - 4
> row_count: 1
> ...
> box.execute([[INSERT INTO tt(t) VALUES (X'3434');]])
> ---
> -- null
> -- 'Type mismatch: can not convert varbinary to string'
> +- autoincrement_ids:
> + - 5
> + row_count: 1
> ...
> box.execute([[SELECT * FROM tt;]])
> ---
> @@ -2360,7 +2367,10 @@ box.execute([[SELECT * FROM tt;]])
> type: string
> rows:
> - [1, null]
> - - [2, '33']
> + - [2, '11']
> + - [3, '22.2']
> + - [4, '33']
> + - [5, '44']
> ...
> box.execute([[INSERT INTO tv(v) VALUES (NULL);]])
> ---
> @@ -2385,13 +2395,14 @@ box.execute([[INSERT INTO tv(v) VALUES (true);]])
> ...
> box.execute([[INSERT INTO tv(v) VALUES ('33');]])
> ---
> -- null
> -- 'Type mismatch: can not convert 33 to varbinary'
> +- autoincrement_ids:
> + - 2
> + row_count: 1
32. Why this does not fails? I think RFC says this this cast is not allowed.
> ...
> box.execute([[INSERT INTO tv(v) VALUES (X'3434');]])
> ---
> - autoincrement_ids:
> - - 2
> + - 3
> row_count: 1
> ...
> box.execute([[SELECT * FROM tv;]])
> @@ -2403,7 +2414,8 @@ box.execute([[SELECT * FROM tv;]])
> type: varbinary
> rows:
> - [1, null]
> - - [2, '44']
> + - [2, '33']
> + - [3, '44']
> ...
> box.execute([[INSERT INTO ts(s) VALUES (NULL);]])
> ---
> @@ -2459,11 +2471,11 @@ box.execute([[SELECT * FROM ts;]])
> -- Check for UPDATE.
> box.execute([[DELETE FROM ti;]])
> ---
> -- row_count: 3
> +- row_count: 4
> ...
> box.execute([[DELETE FROM td;]])
> ---
> -- row_count: 3
> +- row_count: 4
> ...
> box.execute([[DELETE FROM tb;]])
> ---
> @@ -2471,11 +2483,11 @@ box.execute([[DELETE FROM tb;]])
> ...
> box.execute([[DELETE FROM tt;]])
> ---
> -- row_count: 2
> +- row_count: 5
> ...
> box.execute([[DELETE FROM tv;]])
> ---
> -- row_count: 2
> +- row_count: 3
> ...
> box.execute([[DELETE FROM ts;]])
> ---
> @@ -2559,8 +2571,7 @@ box.execute([[UPDATE ti SET i = true WHERE a = 1;]])
> ...
> box.execute([[UPDATE ti SET i = '33' WHERE a = 1;]])
> ---
> -- null
> -- 'Type mismatch: can not convert 33 to integer'
> +- row_count: 1
> ...
> box.execute([[UPDATE ti SET i = X'3434' WHERE a = 1;]])
> ---
> @@ -2600,8 +2611,7 @@ box.execute([[UPDATE td SET d = true WHERE a = 1;]])
> ...
> box.execute([[UPDATE td SET d = '33' WHERE a = 1;]])
> ---
> -- null
> -- 'Type mismatch: can not convert 33 to double'
> +- row_count: 1
> ...
> box.execute([[UPDATE td SET d = X'3434' WHERE a = 1;]])
> ---
> @@ -2616,7 +2626,7 @@ box.execute([[SELECT * FROM td;]])
> - name: D
> type: double
> rows:
> - - [1, 22.2]
> + - [1, 33]
> ...
> box.execute([[UPDATE tb SET b = NULL WHERE a = 1;]])
> ---
> @@ -2662,13 +2672,11 @@ box.execute([[UPDATE tt SET t = NULL WHERE a = 1;]])
> ...
> box.execute([[UPDATE tt SET t = 11 WHERE a = 1;]])
> ---
> -- null
> -- 'Type mismatch: can not convert 11 to string'
> +- row_count: 1
> ...
> box.execute([[UPDATE tt SET t = 22.2 WHERE a = 1;]])
> ---
> -- null
> -- 'Type mismatch: can not convert 22.2 to string'
> +- row_count: 1
> ...
> box.execute([[UPDATE tt SET t = true WHERE a = 1;]])
> ---
> @@ -2681,8 +2689,7 @@ box.execute([[UPDATE tt SET t = '33' WHERE a = 1;]])
> ...
> box.execute([[UPDATE tt SET t = X'3434' WHERE a = 1;]])
> ---
> -- null
> -- 'Type mismatch: can not convert varbinary to string'
> +- row_count: 1
> ...
> box.execute([[SELECT * FROM tt;]])
> ---
> @@ -2692,7 +2699,7 @@ box.execute([[SELECT * FROM tt;]])
> - name: T
> type: string
> rows:
> - - [1, '33']
> + - [1, '44']
> ...
> box.execute([[UPDATE tv SET v = NULL WHERE a = 1;]])
> ---
> @@ -2715,8 +2722,7 @@ box.execute([[UPDATE tv SET v = true WHERE a = 1;]])
> ...
> box.execute([[UPDATE tv SET v = '33' WHERE a = 1;]])
> ---
> -- null
> -- 'Type mismatch: can not convert 33 to varbinary'
> +- row_count: 1
33. Same.
> ...
> box.execute([[UPDATE tv SET v = X'3434' WHERE a = 1;]])
> ---
> --
> 2.29.2
>
next prev parent reply other threads:[~2021-06-20 18:53 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-11 7:48 [Tarantool-patches] [PATCH v2 0/3] sql: modify explicit and implicit conversion tables Timur Safin via Tarantool-patches
2021-06-11 7:48 ` [Tarantool-patches] [PATCH v2 1/3] test: corrected reported error lines Timur Safin via Tarantool-patches
2021-06-20 18:57 ` Igor Munkin via Tarantool-patches
2021-06-23 21:01 ` Alexander Turenko via Tarantool-patches
2021-06-27 23:16 ` Timur Safin via Tarantool-patches
2021-06-29 16:21 ` Igor Munkin via Tarantool-patches
2021-06-30 6:49 ` Timur Safin via Tarantool-patches
2021-07-21 7:24 ` Igor Munkin via Tarantool-patches
2021-06-11 7:48 ` [Tarantool-patches] [PATCH v2 2/3] sql: updated explicit conversion table Timur Safin via Tarantool-patches
2021-06-20 18:52 ` Mergen Imeev via Tarantool-patches
2021-06-25 21:26 ` Timur Safin via Tarantool-patches
2021-06-25 21:26 ` [Tarantool-patches] Отзыв: " Timur Safin via Tarantool-patches
2021-06-27 23:46 ` [Tarantool-patches] " Timur Safin via Tarantool-patches
2021-06-11 7:48 ` [Tarantool-patches] [PATCH v2 3/3] sql: updated implicit " Timur Safin via Tarantool-patches
2021-06-20 18:52 ` Mergen Imeev via Tarantool-patches [this message]
2021-06-28 0:06 ` Timur Safin via Tarantool-patches
2021-06-20 18:52 ` [Tarantool-patches] [PATCH v2 0/3] sql: modify explicit and implicit conversion tables Mergen Imeev via Tarantool-patches
2021-06-27 23:29 ` Timur Safin via Tarantool-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4b33bd96-174f-4749-a749-f07247d05a7a@tarantool.org \
--to=tarantool-patches@dev.tarantool.org \
--cc=imeevma@tarantool.org \
--cc=tsafin@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH v2 3/3] sql: updated implicit conversion table' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox