From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: "n.pettik" <korablev@tarantool.org>, tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH 6/6] sql: allow to specify UNSIGNED column type
Date: Thu, 18 Jul 2019 23:08:35 +0200 [thread overview]
Message-ID: <989f9710-043f-447a-0cc4-76eb317bc1e9@tarantool.org> (raw)
In-Reply-To: <127420CE-540E-439C-B2BD-20007EE98328@tarantool.org>
On 18/07/2019 22:56, n.pettik wrote:
>
>
>> On 18 Jul 2019, at 23:18, Vladislav Shpilevoy <v.shpilevoy@tarantool.org <mailto:v.shpilevoy@tarantool.org>> wrote:
>>
>> Hi!
>>
>> Thanks for the fixes!
>>
>>>> -------------------------
>>>> vdbe.c:307
>>>>
>>>>> case FIELD_TYPE_INTEGER:
>>>>> case FIELD_TYPE_UNSIGNED:
>>>>> if ((record->flags & MEM_Int) == MEM_Int)
>>>>> return 0;
>>>>> if ((record->flags & MEM_UInt) == MEM_UInt)
>>>>> return 0;
>>>>> if ((record->flags & MEM_Real) == MEM_Real) {
>>>>> int64_t i = (int64_t) record->u.r;
>>>>> if (i == record->u.r)
>>>>> mem_set_int(record, record->u.r,
>>>>> record->u.r <= -1);
>>>>> return 0;
>>>>> }
>>>>
>>>> It is a part of function mem_apply_type. When target type is
>>>> UNSIGNED, and a value is MEM_Int, you do nothing. Why? Looks like
>>>> it is possible to pass here a negative value, and CAST UNSIGNED
>>>> would do nothing.
>>>
>>> Basically, this function implements sort of implicit cast
>>> which takes place before comparison/assignment.
>>> For comparisons it makes no sense - we can compare
>>> integer with unsigned value - the latter is always greater.
>>> For assignment it is also meaningless: if we attempt
>>> at inserting negative values to unsigned field appropriate
>>> error will be raised anyway. If you can come up with
>>> specific example, let’s discuss it.
>>>
>>
>> I can't provide a test. But the function is named mem_apply_type,
>> and it doesn't apply type, when it is unsigned, and a value is
>> negative. Doesn't it look wrong to you?
>>
>> If some code wants to get an integer, it can apply FIELD_TYPE_INTEGER
>> instead of FIELD_TYPE_UNSIGNED. IMO, an attempt to apply unsigned
>> to int should raise an error here. Otherwise this function can't
>> be named 'apply_type' because it ignores negative -> unsigned case.
>
> Okay, let’s rename it. I can suggest these options:
>
> mem_cast_implicit()
> mem_cast_implicit_to_type()
> mem_implicit_cast_to_type()
> mem_convert_implicit()
> mem_convert_to_type()
> mem_type_coerce_implicit()
> mem_type_implicit_coercion()
> mem_type_coercion_implicit()
> mem_implicit_type_juggling()
> mem_implicit_juggle_to_type()
> mem_do_implicit_conversion()
> mem_do_implicit_coercion()
>
> Or any other combination :)
>
But it is not implicit. It just does not work, when a value is negative,
and type is unsigned.
next prev parent reply other threads:[~2019-07-18 21:06 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-07 15:37 [tarantool-patches] [PATCH 0/6] Introduce UNSIGNED type in SQL Nikita Pettik
2019-06-07 15:37 ` [tarantool-patches] [PATCH 1/6] sql: refactor sql_atoi64() Nikita Pettik
2019-06-11 21:11 ` [tarantool-patches] " Vladislav Shpilevoy
2019-07-01 14:20 ` n.pettik
2019-07-01 21:53 ` Vladislav Shpilevoy
2019-07-05 16:32 ` n.pettik
2019-06-07 15:37 ` [tarantool-patches] [PATCH 2/6] sql: separate VDBE memory holding positive and negative ints Nikita Pettik
2019-06-11 21:11 ` [tarantool-patches] " Vladislav Shpilevoy
2019-07-01 14:21 ` n.pettik
2019-07-01 21:53 ` Vladislav Shpilevoy
2019-07-05 16:33 ` n.pettik
2019-06-07 15:37 ` [tarantool-patches] [PATCH 3/6] sql: refactor arithmetic operations to support unsigned ints Nikita Pettik
2019-06-11 21:11 ` [tarantool-patches] " Vladislav Shpilevoy
2019-07-01 14:21 ` n.pettik
2019-07-01 21:53 ` Vladislav Shpilevoy
2019-07-05 16:36 ` n.pettik
2019-07-10 22:49 ` Vladislav Shpilevoy
2019-07-17 12:24 ` n.pettik
2019-06-07 15:37 ` [tarantool-patches] [PATCH 4/6] sql: make built-in functions operate on unsigned values Nikita Pettik
2019-06-11 21:11 ` [tarantool-patches] " Vladislav Shpilevoy
2019-07-01 14:21 ` n.pettik
2019-07-01 21:53 ` Vladislav Shpilevoy
2019-07-05 16:36 ` n.pettik
2019-07-10 22:49 ` Vladislav Shpilevoy
2019-07-17 0:53 ` n.pettik
2019-06-07 15:37 ` [tarantool-patches] [PATCH 5/6] sql: introduce extended range for INTEGER type Nikita Pettik
2019-06-11 21:11 ` [tarantool-patches] " Vladislav Shpilevoy
2019-07-01 14:21 ` n.pettik
2019-07-01 21:53 ` Vladislav Shpilevoy
2019-07-24 15:59 ` Konstantin Osipov
2019-07-24 16:54 ` n.pettik
2019-07-24 17:09 ` Konstantin Osipov
2019-06-07 15:37 ` [tarantool-patches] [PATCH 6/6] sql: allow to specify UNSIGNED column type Nikita Pettik
2019-07-01 21:53 ` [tarantool-patches] " Vladislav Shpilevoy
2019-07-05 16:36 ` n.pettik
2019-07-10 22:49 ` Vladislav Shpilevoy
2019-07-11 21:25 ` Vladislav Shpilevoy
2019-07-17 0:53 ` n.pettik
2019-07-18 20:18 ` Vladislav Shpilevoy
2019-07-18 20:56 ` n.pettik
2019-07-18 21:08 ` Vladislav Shpilevoy [this message]
2019-07-18 21:13 ` Vladislav Shpilevoy
2019-07-22 10:20 ` n.pettik
2019-07-22 19:17 ` Vladislav Shpilevoy
2019-07-22 10:20 ` n.pettik
2019-07-17 0:54 ` n.pettik
2019-07-18 20:18 ` Vladislav Shpilevoy
2019-08-06 19:36 ` n.pettik
2019-07-24 13:01 ` [tarantool-patches] Re: [PATCH 0/6] Introduce UNSIGNED type in SQL Kirill Yukhin
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=989f9710-043f-447a-0cc4-76eb317bc1e9@tarantool.org \
--to=v.shpilevoy@tarantool.org \
--cc=korablev@tarantool.org \
--cc=tarantool-patches@freelists.org \
--subject='[tarantool-patches] Re: [PATCH 6/6] sql: allow to specify UNSIGNED column type' \
/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