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.

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.