Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql: wrong types on concatenation w/ RANDOMBLOB() #3544

Closed
kyukhin opened this issue Jul 17, 2018 · 2 comments
Closed

sql: wrong types on concatenation w/ RANDOMBLOB() #3544

kyukhin opened this issue Jul 17, 2018 · 2 comments
Assignees
@Korablev77
Labels
bug Something isn't working crash sql static types sql
Milestone
2.1.2

Comments

@kyukhin
Copy link
Contributor

kyukhin commented Jul 17, 2018

TYPEOF(randomblob(5) || zeroblob(5)) is 'text'.
This is SQLite behaviour -- || always causes text
or null results -- but once again it seems to be a
rule that depends on the operator, not the operands.

box.sql.execute("VALUES (TYPEOF(randomblob(5) || zeroblob(5)));")

Result = 'text'

This is a part of #2347
@kyukhin kyukhin added bug Something isn't working sql labels Jul 17, 2018
@kyukhin kyukhin added this to the 2.1.1 milestone Jul 17, 2018
@Korablev77 Korablev77 self-assigned this Oct 1, 2018
@Korablev77
Copy link
Contributor

Ok, this bug turns out to be severe: zeroblob() usage leads to assertion (sic!):

tarantool> select zeroblob(5)
Assertion failed: (0), function lua_push_row, file /Users/n.pettik/tarantoolOrigin/tarantool/src/box/lua/sql.c, line 61.
Abort trap: 6

I'm not going to open new issue, let's fix it in scope of current problem.

@Korablev77
Copy link
Contributor

For the record: according to ANSI, if one of concatenation operands is of TEXT type, then the rest must have the same type. Otherwise, error is raised. The same for binary-like types. Now concatenation operator ('||') accepts even numeric types. I suggest to re-work concatenation operator to allow only string-like types and validate their type equivalence.

Korablev77 added a commit that referenced this issue Feb 18, 2019
@Korablev77
sql: fix value of mask to map VDBE memory type
c6fc689 
Accidentally, mask which is used to map type of VDBE memory cell into
outer API types was replaced with MEM_TypeMask. But value of the latter
is larger then possible values of VDBE memory cells types. Hence, if it
is applied to memory cell, not only pure types is taken into
consideration, but some additional flags (such as MEM_Zero) as well.
Overall, it results in wrong type calculation for zeroed blobs, for
instance. Lets return back previous mask.

Follow-up #3698
Needed for #3544
Korablev77 added a commit that referenced this issue Feb 18, 2019
@Korablev77
sql: allow only string-like arguments for concatenation
dc3b319 
Original SQLite operator of concatenation accepts all types of
arguments. If type of parameter is not TEXT, it is implicitly converted
to TEXT (except for NULLs). That contradicts ANSI (it is regulated by
[1]), so lets allow only TEXT and BLOB as argument type for
concatenation.  Moreover, they both must be of the same type at the same
time (i.e. both TEXT or BLOB).

[1] SQL ANSI 2013, 9.5 Result of data type combination

Part of #3544
Korablev77 added a commit that referenced this issue Feb 18, 2019
@Korablev77
sql: fix resulting type for concatenation operator
8ed63c9 
Before this patch, resulting type of concatenation operator always was
TEXT (i.e. type of memory cell containing result - MEM_Str). Lets fix it
and return type depending on type of concatenation arguments: if both
arguments are TEXT, then resulting type is TEXT; BLOB otherwise. Note
that other options of combining types of arguments are illegal.

Closes #3544
kyukhin pushed a commit that referenced this issue Feb 25, 2019
@Korablev77 @kyukhin
sql: fix value of mask to map VDBE memory type
17d1c6f 
Accidentally, mask which is used to map type of VDBE memory cell into
outer API types was replaced with MEM_TypeMask. But value of the latter
is larger then possible values of VDBE memory cells types. Hence, if it
is applied to memory cell, not only pure types is taken into
consideration, but some additional flags (such as MEM_Zero) as well.
Overall, it results in wrong type calculation for zeroed blobs, for
instance. Lets return back previous mask.

Follow-up #3698
Needed for #3544
kyukhin pushed a commit that referenced this issue Feb 25, 2019
@Korablev77 @kyukhin
sql: allow only string-like arguments for concatenation
d8cfb80 
Original SQLite operator of concatenation accepts all types of
arguments. If type of parameter is not TEXT, it is implicitly converted
to TEXT (except for NULLs). That contradicts ANSI (it is regulated by
[1]), so lets allow only TEXT and BLOB as argument type for
concatenation.  Moreover, they both must be of the same type at the same
time (i.e. both TEXT or BLOB).

[1] SQL ANSI 2013, 9.5 Result of data type combination

Part of #3544
@kyukhin kyukhin added the tmp label Mar 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Korablev77 Korablev77

Labels
bug Something isn't working crash sql static types sql
Projects
None yet
Milestone
2.1.2
Development

No branches or pull requests
3 participants
@locker @kyukhin @Korablev77