Tarantool development patches archive
 help / color / mirror / Atom feed
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 --]

  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