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

[Mergen Imeev <imeevma@tarantool.org>]

Hi! Thank you for review. My answers and new commit-message
below.

On Wed, Jan 22, 2020 at 04:26:17PM +0300, Nikita Pettik wrote:
> 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?
>  
Can't say for now. It is possible, since now it is
disallowed to explicitly cast from binary to DOUBLE, and
allowed to cast from BINARY to INTEGER.

> > > >     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).
> 
Fixed.

> >     
> >     Closes #4233
> >     Closes #4463

New commit-message:

commit 9982ca9c6e239089e425a16bc5c7b5f9ab847fc4
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.
    
    Examples:
    1) CAST function:
    
    Obsolete behaviour:
    tarantool> SELECT CAST(922337206854774800 AS NUMBER), CAST(5 AS NUMBER) / 10;
    ---
    - metadata:
      - name: CAST(922337206854774800 AS NUMBER)
        type: number
      - name: CAST(5 AS NUMBER) / 10
        type: number
      rows:
      - [922337206854774784, 0.5]
    ...
    
    New behaviour:
    tarantool> SELECT CAST(922337206854774800 AS NUMBER), CAST(5 AS NUMBER) / 10;
    ---
    - metadata:
      - name: CAST(922337206854774800 AS NUMBER)
        type: number
      - name: CAST(5 AS NUMBER) / 10
        type: number
      rows:
      - [922337206854774800, 0]
    ...
    
    2) Table column:
    
    Obsolete behaviour:
    tarantool> CREATE TABLE t (n NUMBER PRIMARY KEY);
    ---
    - row_count: 1
    ...
    
    tarantool> INSERT INTO t VALUES (3), (-4), (5.0);
    ---
    - row_count: 3
    ...
    
    tarantool> SELECT n, n/10 FROM t;
    ---
    - metadata:
      - name: N
        type: number
      - name: n/10
        type: number
      rows:
      - [-4, -0.4]
      - [3, 0.3]
      - [5, 0.5]
    ...
    
    New behaviour:
    tarantool> CREATE TABLE t (n NUMBER PRIMARY KEY);
    ---
    - row_count: 1
    ...
    
    tarantool> INSERT INTO t VALUES (3), (-4), (5.0);
    ---
    - row_count: 3
    ...
    
    tarantool> SELECT n, n/10 FROM t;
    ---
    - metadata:
      - name: N
        type: number
      - name: n/10
        type: number
      rows:
      - [-4, 0]
      - [3, 0]
      - [5, 0.5]
    ...
    
    Closes #4233
    Closes #4463