From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH 6/6] sql: allow to specify UNSIGNED column type
Date: Thu, 18 Jul 2019 23:56:48 +0300 [thread overview]
Message-ID: <127420CE-540E-439C-B2BD-20007EE98328@tarantool.org> (raw)
In-Reply-To: <8e4feefd-7bfb-18af-fd0f-b45384e5d2ef@tarantool.org>
[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]
> On 18 Jul 2019, at 23:18, Vladislav Shpilevoy <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 :)
[-- Attachment #2: Type: text/html, Size: 13183 bytes --]
next prev parent reply other threads:[~2019-07-18 20:56 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 [this message]
2019-07-18 21:08 ` Vladislav Shpilevoy
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=127420CE-540E-439C-B2BD-20007EE98328@tarantool.org \
--to=korablev@tarantool.org \
--cc=tarantool-patches@freelists.org \
--cc=v.shpilevoy@tarantool.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