Tarantool development patches archive
 help / color / mirror / Atom feed
From: Nikita Pettik <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Nikita Pettik <korablev@tarantool.org>
Subject: [tarantool-patches] [PATCH 2/3] sql: remove redundant type derivation from QP
Date: Fri, 24 May 2019 20:39:40 +0300	[thread overview]
Message-ID: <20bccdb8bed2bab9e9eaaca3781e3d0b5b780d10.1558700151.git.korablev@tarantool.org> (raw)
In-Reply-To: <cover.1558700151.git.korablev@tarantool.org>
In-Reply-To: <cover.1558700151.git.korablev@tarantool.org>

Before value to be scanned in index search is passed to the iterator, it
is subjected to implicit type casting (which is implemented by
OP_ApplyType). If value can't be converted to required type,
user-friendly message is raised. Without this cast, type of iterator may
not match with type of key which in turn results in unexpected error.
However, array of types which is used to provide type conversions is
different from types of indexed fields: it is  modified depending on
types of comparison's operands. For instance, when boolean field is
compared with blob value, resulting type is assumed to be scalar. In
turn, conversion to scalar is no-op.  As a result, value with MP_BIN
format is passed to the iterator over boolean field.  To fix that let's
remove this transformation of types. Moreover, it seems to be redundant.

Part of #4187
---
 src/box/sql/wherecode.c | 10 ----------
 test/sql/types.result   | 30 ++++++++++++++++++++++++++++++
 test/sql/types.test.lua | 12 ++++++++++++
 3 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 6f72506ad..977c0fced 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -769,16 +769,6 @@ codeAllEqualityTerms(Parse * pParse,	/* Parsing context */
 						  pLevel->addrBrk);
 				VdbeCoverage(v);
 			}
-			if (type != NULL) {
-				enum field_type rhs_type =
-					sql_expr_type(pRight);
-				if (sql_type_result(rhs_type, type[j]) ==
-				    FIELD_TYPE_SCALAR) {
-					type[j] = FIELD_TYPE_SCALAR;
-				}
-				if (sql_expr_needs_no_type_change(pRight, type[j]))
-					type[j] = FIELD_TYPE_SCALAR;
-			}
 		}
 	}
 	*res_type = type;
diff --git a/test/sql/types.result b/test/sql/types.result
index bc4518c01..758018eb5 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -897,3 +897,33 @@ box.execute('SELECT ?', {true})
   rows:
   - [true]
 ...
+-- gh-4187: make sure that value passsed to the iterator has
+-- the same type as indexed fields.
+--
+box.execute("CREATE TABLE tboolean (s1 BOOLEAN PRIMARY KEY);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO tboolean VALUES (TRUE);")
+---
+- row_count: 1
+...
+box.execute("SELECT * FROM tboolean WHERE s1 = x'44';")
+---
+- error: 'Type mismatch: can not convert D to boolean'
+...
+box.execute("SELECT * FROM tboolean WHERE s1 = 'abc';")
+---
+- error: 'Type mismatch: can not convert abc to boolean'
+...
+box.execute("SELECT * FROM tboolean WHERE s1 = 1;")
+---
+- error: 'Type mismatch: can not convert INTEGER to boolean'
+...
+box.execute("SELECT * FROM tboolean WHERE s1 = 1.123;")
+---
+- error: 'Type mismatch: can not convert REAL to boolean'
+...
+box.space.TBOOLEAN:drop()
+---
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index c51660cb9..3c7bd5ea3 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -216,3 +216,15 @@ box.execute('SELECT \'9223372036854\' + 1;')
 
 -- Fix BOOLEAN bindings.
 box.execute('SELECT ?', {true})
+
+-- gh-4187: make sure that value passsed to the iterator has
+-- the same type as indexed fields.
+--
+box.execute("CREATE TABLE tboolean (s1 BOOLEAN PRIMARY KEY);")
+box.execute("INSERT INTO tboolean VALUES (TRUE);")
+box.execute("SELECT * FROM tboolean WHERE s1 = x'44';")
+box.execute("SELECT * FROM tboolean WHERE s1 = 'abc';")
+box.execute("SELECT * FROM tboolean WHERE s1 = 1;")
+box.execute("SELECT * FROM tboolean WHERE s1 = 1.123;")
+
+box.space.TBOOLEAN:drop()
-- 
2.15.1

  parent reply	other threads:[~2019-05-24 17:39 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-24 17:39 [tarantool-patches] [PATCH 0/3] Fix passing key with different from iterator's type Nikita Pettik
2019-05-24 17:39 ` [tarantool-patches] [PATCH 1/3] sql: remove redundant check of space format from QP Nikita Pettik
2019-05-24 17:39 ` Nikita Pettik [this message]
2019-05-27 21:49   ` [tarantool-patches] Re: [PATCH 2/3] sql: remove redundant type derivation " Vladislav Shpilevoy
2019-05-28 19:58     ` n.pettik
2019-05-24 17:39 ` [tarantool-patches] [PATCH 3/3] sql: fix passing FP values to integer iterator Nikita Pettik
2019-05-25  5:51   ` [tarantool-patches] " Konstantin Osipov
2019-05-27 21:49     ` Vladislav Shpilevoy
2019-05-28  7:19       ` Konstantin Osipov
2019-05-28 11:31         ` Vladislav Shpilevoy
2019-05-29  7:02           ` Konstantin Osipov
2019-05-28 13:00         ` n.pettik
2019-05-29  9:14           ` Konstantin Osipov
2019-05-27 21:49   ` Vladislav Shpilevoy
2019-05-28 19:58     ` n.pettik
2019-06-02 18:11       ` Vladislav Shpilevoy
2019-06-06 13:51         ` n.pettik
2019-06-06 19:07           ` Vladislav Shpilevoy
2019-07-11  9:19 ` [tarantool-patches] Re: [PATCH 0/3] Fix passing key with different from iterator's type Kirill Yukhin

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=20bccdb8bed2bab9e9eaaca3781e3d0b5b780d10.1558700151.git.korablev@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH 2/3] sql: remove redundant type derivation from QP' \
    /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