> On 16/01/2019 17:26, n.pettik wrote: >>>> Numeric affinity in SQLite means the same as real, except that it >>>> forces integer values into floating point representation in case >>>> it can be converted without loss (e.g. 2.0 -> 2). >>>> Since in Tarantool core there is no difference between numeric and real >>>> values (both are stored as values of type NUMBER), lets remove numeric >>>> affinity and use instead real. >>>> The only real pitfall is implicit conversion mentioned above. >>>> What is more, vinyl engine complicates problem since it relies >>>> on data encoding (i.e. whether it is encoded as MP_INT or MP_FLOAT). >>>> For instance, if we encode 1.0 as MP_FLOAT during insertion, we won't >>>> be able to use iterators from Lua, since they implicitly change type of >>>> 1.0 and pass it to the iterator as MP_INT. Solution to this problem is >>>> simple: lets always attempt at encoding floats as ints if conversion >>>> takes place without loss. This is a straightforward approach, but to >>>> implement it we need to care about reversed (decoding) situation. >>> >>> The bug is confirmed: https://github.com/tarantool/tarantool/issues/3907 >>> >>> I agree with Kostja - it is just a bug, that Vinyl treats differently >>> integers and their double casts. It should not affect design decisions >>> of this patchset. >> Ok, we may consider part of this patch as a workaround. >> Now affinity removal would significantly help me to fix >> other issues connected with strict typing (e.g. implicit casts). >> afterwards code introduced in this commit may be simplified/removed. >> I don’t know what priority of #3907 issue (milestone is 2.1.1 but >> we know that sometimes it may take a while). > > As we agreed, it is just a Vinyl bug, and now this part of the > commit message looks strange: > > " > The only real pitfall is implicit conversion mentioned above. > What is more, vinyl engine complicates problem since it relies > on data encoding (i.e. whether it is encoded as MP_INT or MP_FLOAT). > For instance, if we encode 1.0 as MP_FLOAT during insertion, we won't > be able to use iterators from Lua, since they implicitly change type of > 1.0 and pass it to the iterator as MP_INT. > " > > Here you've stated that Vinyl even for number indexes sees a difference > between 2.0 and 2, but it is wrong (in an ideal world, but in our it is > just a bug). It is better to write here about not a temporary bug, but > about, for instance, the example I've showed you below. > >>> But there is another reason why we can't pass *.0 as an iterator value - >>> our fast comparators (TupleCompare, TupleCompareWithKey) are designed to >>> work with only values of same MP_ type. They do not use slow >>> tuple_compare_field() which is able to compare double and integer. >> Yep, it is sad. I can’t say anything more now,I need to "think about it”. > > Please, use this example or find another to support your decision > always 'integerifying' float numbers having zero fraction. Ok, I replaced original paragraph with yours. >>>> diff --git a/src/box/sql.c b/src/box/sql.c >>>> index a06c50dca..a498cd8fe 100644 >>>> --- a/src/box/sql.c >>>> +++ b/src/box/sql.c >>>> @@ -376,14 +376,18 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info) >>>> for (uint32_t i = 0; i < field_count; ++i) { >>>> struct key_part_def *part = &ephemer_key_parts[i]; >>>> part->fieldno = i; >>>> - part->type = FIELD_TYPE_SCALAR; >>>> part->nullable_action = ON_CONFLICT_ACTION_NONE; >>>> part->is_nullable = true; >>>> part->sort_order = SORT_ORDER_ASC; >>>> - if (def != NULL && i < def->part_count) >>>> + if (def != NULL && i < def->part_count) { >>>> + assert(def->parts[i].type < field_type_MAX); >>>> + part->type = def->parts[i].type != FIELD_TYPE_ANY ? >>>> + def->parts[i].type : FIELD_TYPE_SCALAR; >>>> part->coll_id = def->parts[i].coll_id; >>> >>> 1. How can key_part have FIELD_TYPE_ANY? We have no comparators for ANY >>> type, it is impossible, isn't it? >> We don’t, and that is why I replace ANY with SCALAR. > > No, you still check for "def->parts[i].type != FIELD_TYPE_ANY", and I > can not understand how is it possible. struct key_def can not have > FIELD_TYPE_ANY in its parts. Now this problem is fixed in the next patches. In this one it can’t be fixed with ease.