Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mergen Imeev <imeevma@tarantool.org>
To: Nikita Pettik <korablev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 1/3] sql: allow conversion of numeric binary values to DOUBLE
Date: Tue, 31 Dec 2019 11:16:23 +0300	[thread overview]
Message-ID: <20191231081623.GA21542@tarantool.org> (raw)
In-Reply-To: <20191230131749.GB74649@tarantool.org>

Hi! Thank you for review. You are right, I dropped this
commit. All implicit/explicit casts will be fixed in the
other issues (such as #3809, 4230 and so on).

On Mon, Dec 30, 2019 at 03:17:49PM +0200, Nikita Pettik wrote:
> On 30 Dec 16:01, imeevma@tarantool.org wrote:
> > This patch allows to convert binary values consisting of numeric
> > literals to numbers.
> 
> What is the justification for this change? I see no reason to allow
> implicitly casting blobs to floating points.
> 
> > Follow-uo #3812
> 
> -> up
> 
> > ---
> >  src/box/sql/vdbemem.c                |  2 +-
> >  test/sql-tap/numcast.test.lua        | 25 ++++++++++++++++++++++++-
> >  test/sql-tap/tkt-80e031a00f.test.lua |  8 ++++----
> >  test/sql/types.result                |  7 +++++--
> >  4 files changed, 34 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
> > index df3f0d8..3c5e5fc 100644
> > --- a/src/box/sql/vdbemem.c
> > +++ b/src/box/sql/vdbemem.c
> > @@ -516,7 +516,7 @@ sqlVdbeRealValue(Mem * pMem, double *v)
> >  	} else if ((pMem->flags & MEM_UInt) != 0) {
> >  		*v = (double)pMem->u.u;
> >  		return 0;
> > -	} else if (pMem->flags & MEM_Str) {
> > +	} else if ((pMem->flags & (MEM_Blob | MEM_Str)) != 0) {
> >  		if (sqlAtoF(pMem->z, v, pMem->n))
> >  			return 0;
> >  	}
> > diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
> > index 07117d0..a45daef 100755
> > --- a/test/sql-tap/numcast.test.lua
> > +++ b/test/sql-tap/numcast.test.lua
> > @@ -1,6 +1,6 @@
> >  #!/usr/bin/env tarantool
> >  test = require("sqltester")
> > -test:plan(20)
> > +test:plan(22)
> >  
> >  --!./tcltestrunner.lua
> >  -- 2013 March 20
> > @@ -147,4 +147,27 @@ test:do_catchsql_test(
> >          1,"Tuple field 1 type does not match one required by operation: expected integer"
> >      })
> >  
> > +--
> > +-- Allow to convert binary values consisting of numeric literals
> > +-- to numbers.
> > +--
> > +test:do_execsql_test(
> > +    "cast-3.1",
> > +    [[
> > +        CREATE TABLE td (i DOUBLE PRIMARY KEY);
> > +        INSERT INTO td VALUES(X'3132332E35');
> > +        INSERT INTO td VALUES(X'31323334');
> > +        SELECT * FROM td;
> > +    ]], {
> > +        123.5, 1234
> > +    })
> > +
> > +test:do_execsql_test(
> > +    "cast-3.2",
> > +    [[
> > +        SELECT CAST(X'39' AS DOUBLE), CAST(X'39' AS DOUBLE) / 10;
> > +    ]], {
> > +        9, 0.9
> > +    })
> > +
> >  test:finish_test()
> > diff --git a/test/sql-tap/tkt-80e031a00f.test.lua b/test/sql-tap/tkt-80e031a00f.test.lua
> > index a0e6539..01f4265 100755
> > --- a/test/sql-tap/tkt-80e031a00f.test.lua
> > +++ b/test/sql-tap/tkt-80e031a00f.test.lua
> > @@ -380,23 +380,23 @@ test:do_execsql_test(
> >          -- </tkt-80e031a00f.30>
> >      })
> >  
> > -test:do_catchsql_test(
> > +test:do_execsql_test(
> >      "tkt-80e031a00f.31",
> >      [[
> >          SELECT x'303132' IN t1
> >      ]], {
> >          -- <tkt-80e031a00f.31>
> > -        1, 'Type mismatch: can not convert varbinary to integer'
> > +        false
> >          -- </tkt-80e031a00f.31>
> >      })
> >  
> > -test:do_catchsql_test(
> > +test:do_execsql_test(
> >      "tkt-80e031a00f.32",
> >      [[
> >          SELECT x'303132' NOT IN t1
> >      ]], {
> >          -- <tkt-80e031a00f.32>
> > -        1, 'Type mismatch: can not convert varbinary to integer'
> > +        true
> >          -- </tkt-80e031a00f.32>
> >      })
> >  
> > diff --git a/test/sql/types.result b/test/sql/types.result
> > index 6d0aefd..8c0186d 100644
> > --- a/test/sql/types.result
> > +++ b/test/sql/types.result
> > @@ -1760,8 +1760,11 @@ box.execute("SELECT CAST(x'' AS DOUBLE);")
> >  ...
> >  box.execute("SELECT CAST(x'35' AS DOUBLE);")
> >  ---
> > -- null
> > -- 'Type mismatch: can not convert varbinary to double'
> > +- metadata:
> > +  - name: CAST(x'35' AS DOUBLE)
> > +    type: double
> > +  rows:
> > +  - [5]
> >  ...
> >  box.execute("SELECT CAST(CAST(x'35' AS STRING) AS DOUBLE);")
> >  ---
> > -- 
> > 2.7.4
> > 

  reply	other threads:[~2019-12-31  8:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-30 13:01 [Tarantool-patches] [PATCH v2 0/3] sql: fix NUMBER type imeevma
2019-12-30 13:01 ` [Tarantool-patches] [PATCH v2 1/3] sql: allow conversion of numeric binary values to DOUBLE imeevma
2019-12-30 13:17   ` Nikita Pettik
2019-12-31  8:16     ` Mergen Imeev [this message]
2019-12-30 13:01 ` [Tarantool-patches] [PATCH v2 2/3] sql: fix typeof() for double values imeevma
2019-12-30 13:01 ` [Tarantool-patches] [PATCH v2 3/3] sql: make NUMBER to be union of SQL numeric types imeevma

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=20191231081623.GA21542@tarantool.org \
    --to=imeevma@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 1/3] sql: allow conversion of numeric binary values to DOUBLE' \
    /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