Re: [Tarantool-patches] [PATCH v2 1/1] sql: make NUMBER to be union of SQL numeric types

[Nikita Pettik <korablev@tarantool.org>]

On 17 Jan 11:06, Mergen Imeev wrote:
> Hi! Thank you for review. My answer and new commit-message
> below.
> 
> > > commit 492646da4f781b863e5e3e488a0be53f31d28964
> > > Author: Mergen Imeev <imeevma@gmail.com>
> > > Date:   Sat Oct 26 17:27:53 2019 +0300
> > > 
> > >     sql: make NUMBER to be union of SQL numeric types
> > >     
> > >     This patch makes number to be union of UNSIGNED, INTEGER and
> > >     DOUBLE numeric types.
> > >     
> > >     Closes #4233
> > >     Closes #4463
> > >     
> > >     @TarantoolBot document
> > >     Title: NUMBER type in SQL.
> > 
> > Could you please intead of 3 paragraphs just include following
> > changelog list:
> > 
> > "This patch makes definition of NUMBER type be consistent with NoSQL.
> >  Previously (for historic reasons) in half-cases NUMBER served as a
> >  synonym of DOUBLE type.
> >  After patch is applied following will change:
> >  - CAST AS NUMBER operation no more longer results in any value
> >    change in case value is already of numeric type. Example:
> >    Obsolete behaviour:
> > 	SELECT CAST(922337206854774800 AS NUMBER);
> >         Result: 9223372036854774784
> >    New behaviour:
> >         SELECT CAST(922337206854774800 AS NUMBER);                              
> >         Result: 9223372036854774800LL
> >  - ...
> > "
> > 
> > Or group implicit/explicit cast changes. Text below is quite
> > compicated to perceive (IMHO).
> > 
> I removed the part that describes the cast from STRING and
> VARBINARY to NUMBER, since I'm not sure if this is the
> correct behavior. I think it will be fixed in #3809 and
> #4230 issues:
> https://github.com/tarantool/tarantool/issues/3809
> https://github.com/tarantool/tarantool/issues/4230

Does this statement imply any code changes?
 
> > >     The NUMBER type in SQL is the union of all currently available
> > >     numeric types in SQL: INTEGER, UNSIGNED, and DOUBLE. The types
> > >     INTEGER and DOUBLE can be called subtypes of type NUMBER. Any
> > >     value of type NUMBER has a subtype of INTEGER or DOUBLE. When
> > >     any numeric value is implicitly or explicitly casted to NUMBER,
> > >     the only thing that can change is its type. Its type will become
> > >     NUMBER. When a value of type NUMBER is cast explicitly or
> > >     implicitly to other numeric types, the rules applicable to the
> > >     cast are determined by the subtype of the value.
> > >     
> > >     If a value of type STRING can be implicitly cast to value of
> > >     type INTEGER or DOUBLE, then this value can be cast explicitly
> > >     and implicitly to a value of type NUMBER. If this value can be
> > >     implicitly cast to INTEGER, then its subtype will be INTEGER.
> > >     The subtype will be DOUBLE in another case.
> > >     
> > >     If a value of type VARBINARY can be explicitly cast to type
> > >     INTEGER or DOUBLE, then this value can be explicitly cast to a
> > >     value of type NUMBER. If this value can be explicitly cast to
> > >     INTEGER, then its subtype will be INTEGER. The subtype will be
> > >     DOUBLE in another case.
> > > > > ---
> 
> New commit-message:
> 
> commit ac5e4fcffe17767efa7da77427024e2d679bebf7
> Author: Mergen Imeev <imeevma@gmail.com>
> Date:   Sat Oct 26 17:27:53 2019 +0300
> 
>     sql: make NUMBER to be union of SQL numeric types
>     
>     This patch makes definition of NUMBER type be consistent with
>     NoSQL. Previously (for historic reasons) in half-cases NUMBER
>     served as a synonym of DOUBLE type.
>     
>     After patch is applied CAST AS NUMBER operation no more longer
>     results in any value change in case value is already of numeric
>     type.
>     
>     Example:
>     Obsolete behaviour:
>     	SELECT CAST(922337206854774800 AS NUMBER);
>     	Result: 9223372036854774784
>     New behaviour:
>     	SELECT CAST(922337206854774800 AS NUMBER);
>     	Result: 9223372036854774800LL

Is CAST the only operation is affected in scope of your patch?
According to test changes - no. Please, provide an example on each
user-visible change (you might not notice ellipsis in my remark).

>     
>     Closes #4233
>     Closes #4463