Tarantool discussions archive
 help / color / mirror / Atom feed
From: Peter Gulutzan <pgulutzan@ocelot.ca>
To: Mergen Imeev <imeevma@tarantool.org>,
	tarantool-discussions@dev.tarantool.org,
	Nikita Pettik <korablev@tarantool.org>,
	Vladislav Shpilevoy <v.shpilevoy@tarantool.org>,
	tsafin@tarantool.org
Subject: Re: [Tarantool-discussions] Check implicit cast between strings and numbers
Date: Mon, 6 Jul 2020 13:51:48 -0600	[thread overview]
Message-ID: <1924f068-8ec2-ad91-569a-7eeecaad9767@ocelot.ca> (raw)
In-Reply-To: <2a8578c7-10f5-fd38-409e-4e996f07b865@tarantool.org>

Hi,


On 2020-07-06 7:52 a.m., Mergen Imeev wrote:
> Hi,
>
> Peter, could you please take a look at my branch and say if
> something is wrong with the implicit casts between strings and
> numerics. I mean, there shouldn't be any.
>
> Branch: imeevma/gh-3809-disallow-imlicit-cast-from-string-to-nums
>
> There should be no implicit casts for assignment after this patch,
> except for the following:
> 1) Any type can be implicitly cast to ANY.
> 2) Any scalar type can be implicitly cast to SCALAR.
> 3) Any numeric type can be implicitly cast to NUMBER.
> 4) In some cases, numbers can be implicitly converted to another
> number type. The rules can be seen in the commit message of the
> patches.
>
> For comparison, we say that any numbers can be compared with each
> other. However, you may find some cases where the result is not
> what it should be. This will be fixed in another issue.
>
> Also, comparison with SCALAR does not work correctly. This will
> also be fixed later after we decide how this should work.
>
Hi,

I looked. I found nothing significant.
Some of my statements here may be repeitions of earlier statements.

Some tests with arithmetic: These are examples in the version 2.4 manual:
box.execute([[select '7' + '7' /* result is 14, metadata = scalar */;]])
box.execute([[SELECT 5 / '5' /* result is 1 */;]])
I assume that this will be fixed soon.

Some tests of functions:
box.execute([[SELECT ABS('') /* result is 0 */;]])
box.execute([[SELECT CAST('' AS DOUBLE) /* result is error */;]])
box.execute([[SELECT CHAR('') /* result is '\0' */;]])
box.execute([[SELECT GREATEST('',1e500,X'00',0,CAST(100 AS DOUBLE));]])
box.execute([[SELECT LENGTH(1234.56) /* result is 7 */;]])
box.execute([[SELECT LIKELIHOOD('a' = 'b', '0.1') /* result is error */;]])
box.execute([[SELECT LOWER(5) /* result is '5' */;]])
box.execute([[SELECT POSITION(14, 3.14159) /* result is error */;]])
box.execute([[SELECT QUOTE(5) /* result is 5 */;]])
box.execute([[SELECT RANDOMBLOB('') /* result is null */;]])
box.execute([[SELECT ROUND('44.7') /* result is 45 */;]])
box.execute([[SELECT SOUNDEX(14) /* result is */;;}
box.execute([[SELECT SOUNDEX(14) /* result is ?000 */;]])
box.execute([[SELECT SUBSTR(5,5,5) /* result is '' */;]])
box.execute([[SELECT TRIM('1' from 121) /* result is 2 */;]])
box.execute([[SELECT UNICODE(0) /* result is 48 */;]])
box.execute([[SELECT UPPER(5) /* result is '5' */;]])
box.execute([[SELECT ZEROBLOB('1') /* result is '/0' */;]])
I see that sometimes strings are accepted where numbers are expected.
I see that sometimes numbers are accepted where strings are expected.
Maybe this isn't always bad, but maybe you intend to make changes.

A test with -nan:
box.execute([[DROP TABLE t;]])
box.execute([[CREATE TABLE t (s1 INTEGER PRIMARY KEY, s2 DOUBLE, s3 
INTEGER);]])
box.space.T:insert{1, ffi.cast('double',math.sqrt(-1)),0}
box.execute([[UPDATE t SET s3 = s2;]])
This succeeds, column s3 becomes NULL.
I guess this is an "implicit cast" of -nan to NULL, I think that is okay.

A test with foreign keys:
box.execute([[DROP TABLE t2;]])
box.execute([[DROP TABLE t1;]])
box.execute([[CREATE TABLE t1 (s1 DOUBLE PRIMARY KEY);]])
box.execute([[CREATE TABLE t2 (s1 INTEGER PRIMARY KEY REFERENCES t1);]])
This fails. The standard requirement is
"The declared type of each referencing column shall be comparable
to the declared type of the corresponding referenced column."
So if it is easy to declare that the statement is legal,
you would be very slightly more compliant with the standard.

A test with a numeric target:
"
tarantool> box.execute([[CREATE TABLE t (s1 INTEGER PRIMARY KEY);]])
---
- row_count: 1
...

tarantool> box.execute([[INSERT INTO t VALUES ('0');]])
---
- null
- 'Type mismatch: can not convert ''0'' (type: text) to integer'
...
"
Fine, but it might be better if the error message said "type: string".

A test with a constraint:
box.execute([[DROP TABLE t4;]])
box.execute([[CREATE TABLE t4 (s1 INTEGER PRIMARY KEY);]])
box.execute([[ALTER TABLE t4 ADD CONSTRAINT c CHECK (s1 <> '');]])
box.execute([[INSERT INTO t4 VALUES (0);]])
The result is an error. Fine.
I wonder whether it would be easy to check during ALTER rather than 
during INSERT.

A test with LIMIT:
n = -1
box.execute([[SELECT 5 LIMIT ?;]],{n})
n = 1
s = box.prepare([[SELECT 5 LIMIT ?;]])
box.execute(s.stmt_id,{n})
n = -1
box.execute(s.stmt_id,{n})
This is a situation that I worried about. It works as planned. Fine.

As you know, we do not agree about SCALAR.

If you are sure that this behaviour change will be in version 2.5,
then please order me or Elena Shebunyaeva to put a warning in
version 2.4 release notes
https://www.tarantool.io/en/doc/2.4/whats_new/
or in the SQL section which mentions "implicit casting" several times.

Peter Gulutzan

  reply	other threads:[~2020-07-06 19:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 13:52 Mergen Imeev
2020-07-06 19:51 ` Peter Gulutzan [this message]
2020-07-08 16:58   ` Mergen Imeev
2020-07-08 18:58     ` Peter Gulutzan

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=1924f068-8ec2-ad91-569a-7eeecaad9767@ocelot.ca \
    --to=pgulutzan@ocelot.ca \
    --cc=imeevma@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-discussions@dev.tarantool.org \
    --cc=tsafin@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-discussions] Check implicit cast between strings and numbers' \
    /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