Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL
@ 2021-10-19  6:19 Mergen Imeev via Tarantool-patches
  2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 1/3] sql: fix truncation of DECIMAL in implicit cast Mergen Imeev via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-10-19  6:19 UTC (permalink / raw)
  To: kyukhin; +Cc: tarantool-patches

This patch-set fixes three bugs with DECIMAL in SQL.

https://github.com/tarantool/tarantool/issues/6485
https://github.com/tarantool/tarantool/tree/imeevma/gh-6485-fix-bugs-with-decimal

Mergen Imeev (3):
  sql: fix truncation of DECIMAL in implicit cast
  sql: fix cast of small negative DECIMAL to INTEGER
  sql: do not truncate DECIMAL in LIMIT and OFFSET

 .../unreleased/gh-6485-bug-of-decimal.md      |  8 +++
 src/box/sql/mem.c                             | 16 +++---
 test/sql-tap/engine.cfg                       |  1 +
 test/sql-tap/gh-6485-bugs-in-decimal.test.lua | 57 +++++++++++++++++++
 4 files changed, 75 insertions(+), 7 deletions(-)
 create mode 100644 changelogs/unreleased/gh-6485-bug-of-decimal.md
 create mode 100755 test/sql-tap/gh-6485-bugs-in-decimal.test.lua

-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Tarantool-patches] [PATCH v1 1/3] sql: fix truncation of DECIMAL in implicit cast
  2021-10-19  6:19 [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL Mergen Imeev via Tarantool-patches
@ 2021-10-19  6:19 ` Mergen Imeev via Tarantool-patches
  2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 2/3] sql: fix cast of small negative DECIMAL to INTEGER Mergen Imeev via Tarantool-patches
  2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 3/3] sql: do not truncate DECIMAL in LIMIT and OFFSET Mergen Imeev via Tarantool-patches
  2 siblings, 0 replies; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-10-19  6:19 UTC (permalink / raw)
  To: kyukhin; +Cc: tarantool-patches

In case the DECIMAL value is implicitly cast to INTEGER during a search
using an index, it was possible that DECIMAL would be truncated, which
is not correct according to the implicit cast rules. This patch removes
this truncation.

Part of #6485
---
 src/box/sql/mem.c                             |  5 +++--
 test/sql-tap/engine.cfg                       |  1 +
 test/sql-tap/gh-6485-bugs-in-decimal.test.lua | 16 ++++++++++++++++
 3 files changed, 20 insertions(+), 2 deletions(-)
 create mode 100755 test/sql-tap/gh-6485-bugs-in-decimal.test.lua

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 5e23c901c..b0eba303e 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -1134,6 +1134,7 @@ static inline int
 dec_to_int_forced(struct Mem *mem)
 {
 	assert(mem->type == MEM_TYPE_DEC);
+	bool is_dec_int = decimal_is_int(&mem->u.d);
 	if (decimal_is_neg(&mem->u.d)) {
 		int64_t i;
 		mem->type = MEM_TYPE_INT;
@@ -1148,7 +1149,7 @@ dec_to_int_forced(struct Mem *mem)
 		 * Decimal is floored when cast to int, which means that after
 		 * cast it becomes bigger if it was not integer.
 		 */
-		return decimal_is_int(&mem->u.d) ? 0 : -1;
+		return is_dec_int ? 0 : -1;
 	}
 	uint64_t u;
 	mem->type = MEM_TYPE_UINT;
@@ -1162,7 +1163,7 @@ dec_to_int_forced(struct Mem *mem)
 	 * Decimal is floored when cast to uint, which means that after cast it
 	 * becomes less if it was not integer.
 	 */
-	return decimal_is_int(&mem->u.d) ? 0 : 1;
+	return is_dec_int ? 0 : 1;
 }
 
 static inline int
diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
index a6f03307f..9d318a456 100644
--- a/test/sql-tap/engine.cfg
+++ b/test/sql-tap/engine.cfg
@@ -39,6 +39,7 @@
     "gh-6376-wrong-double-to-dec-cmp.test.lua": {},
     "gh-4077-iproto-execute-no-bind.test.lua": {},
     "gh-6375-assert-on-unsupported-ext.test.lua": {},
+    "gh-6485-bugs-in-decimal.test.lua": {},
     "*": {
         "memtx": {"engine": "memtx"},
         "vinyl": {"engine": "vinyl"}
diff --git a/test/sql-tap/gh-6485-bugs-in-decimal.test.lua b/test/sql-tap/gh-6485-bugs-in-decimal.test.lua
new file mode 100755
index 000000000..3f63f2b76
--- /dev/null
+++ b/test/sql-tap/gh-6485-bugs-in-decimal.test.lua
@@ -0,0 +1,16 @@
+#!/usr/bin/env tarantool
+local test = require("sqltester")
+test:plan(1)
+
+-- Make sure DECIMAL is not truncated when used in an index.
+test:do_execsql_test(
+    "gh-6485-1",
+    [[
+        CREATE TABLE t(i INTEGER PRIMARY KEY);
+        INSERT INTO t VALUES(1), (2);
+        SELECT i FROM t WHERE i IN (CAST(1.1 AS DECIMAL), CAST(2.2 AS DECIMAL));
+        DROP TABLE t;
+    ]], {
+    })
+
+test:finish_test()
-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Tarantool-patches] [PATCH v1 2/3] sql: fix cast of small negative DECIMAL to INTEGER
  2021-10-19  6:19 [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL Mergen Imeev via Tarantool-patches
  2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 1/3] sql: fix truncation of DECIMAL in implicit cast Mergen Imeev via Tarantool-patches
@ 2021-10-19  6:19 ` Mergen Imeev via Tarantool-patches
  2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 3/3] sql: do not truncate DECIMAL in LIMIT and OFFSET Mergen Imeev via Tarantool-patches
  2 siblings, 0 replies; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-10-19  6:19 UTC (permalink / raw)
  To: kyukhin; +Cc: tarantool-patches

This patch fixes an assertion when casting DECIMAL value less than 0 and
greater than -1 to INTEGER.

Part of #6485
---
 src/box/sql/mem.c                             |  9 +++----
 test/sql-tap/gh-6485-bugs-in-decimal.test.lua | 24 ++++++++++++++++++-
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index b0eba303e..bc6f9b35a 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -1106,9 +1106,9 @@ dec_to_int(struct Mem *mem)
 		int64_t i;
 		if (decimal_to_int64(&mem->u.d, &i) == NULL)
 			return -1;
-		assert(i < 0);
+		assert(i <= 0);
 		mem->u.i = i;
-		mem->type = MEM_TYPE_INT;
+		mem->type = i == 0 ? MEM_TYPE_UINT : MEM_TYPE_INT;
 		mem->flags = 0;
 		return 0;
 	}
@@ -1137,14 +1137,15 @@ dec_to_int_forced(struct Mem *mem)
 	bool is_dec_int = decimal_is_int(&mem->u.d);
 	if (decimal_is_neg(&mem->u.d)) {
 		int64_t i;
-		mem->type = MEM_TYPE_INT;
 		mem->flags = 0;
 		if (decimal_to_int64(&mem->u.d, &i) == NULL) {
 			mem->u.i = INT64_MIN;
+			mem->type = MEM_TYPE_INT;
 			return -1;
 		}
-		assert(i < 0);
+		assert(i <= 0);
 		mem->u.i = i;
+		mem->type = i == 0 ? MEM_TYPE_UINT : MEM_TYPE_INT;
 		/*
 		 * Decimal is floored when cast to int, which means that after
 		 * cast it becomes bigger if it was not integer.
diff --git a/test/sql-tap/gh-6485-bugs-in-decimal.test.lua b/test/sql-tap/gh-6485-bugs-in-decimal.test.lua
index 3f63f2b76..0b9b2ea0a 100755
--- a/test/sql-tap/gh-6485-bugs-in-decimal.test.lua
+++ b/test/sql-tap/gh-6485-bugs-in-decimal.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(1)
+test:plan(3)
 
 -- Make sure DECIMAL is not truncated when used in an index.
 test:do_execsql_test(
@@ -13,4 +13,26 @@ test:do_execsql_test(
     ]], {
     })
 
+--
+-- Make sure that DECIMAL greater than -1 and less than 0 are correctly cast to
+-- INTEGER.
+--
+test:do_execsql_test(
+    "gh-6485-2",
+    [[
+        SELECT CAST(CAST(-0.5 AS DECIMAL) AS INTEGER);
+    ]], {
+        0
+    })
+
+test:do_execsql_test(
+    "gh-6485-3",
+    [[
+        CREATE TABLE t(i INTEGER PRIMARY KEY);
+        INSERT INTO t VALUES(1);
+        SELECT i FROM t WHERE i IN (CAST(-0.1 AS DECIMAL), CAST(2 AS DECIMAL));
+        DROP TABLE t;
+    ]], {
+    })
+
 test:finish_test()
-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Tarantool-patches] [PATCH v1 3/3] sql: do not truncate DECIMAL in LIMIT and OFFSET
  2021-10-19  6:19 [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL Mergen Imeev via Tarantool-patches
  2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 1/3] sql: fix truncation of DECIMAL in implicit cast Mergen Imeev via Tarantool-patches
  2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 2/3] sql: fix cast of small negative DECIMAL to INTEGER Mergen Imeev via Tarantool-patches
@ 2021-10-19  6:19 ` Mergen Imeev via Tarantool-patches
  2 siblings, 0 replies; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-10-19  6:19 UTC (permalink / raw)
  To: kyukhin; +Cc: tarantool-patches

This patch removes the DECIMAL truncation in LIMIT and OFFSET, because
according to the implicit casting rules, DECIMAL with digits after the
decimal point cannot be implicitly cast to INTEGER.

Closes #6485
---
 .../unreleased/gh-6485-bug-of-decimal.md      |  8 +++++++
 src/box/sql/mem.c                             |  2 +-
 test/sql-tap/gh-6485-bugs-in-decimal.test.lua | 21 ++++++++++++++++++-
 3 files changed, 29 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/unreleased/gh-6485-bug-of-decimal.md

diff --git a/changelogs/unreleased/gh-6485-bug-of-decimal.md b/changelogs/unreleased/gh-6485-bug-of-decimal.md
new file mode 100644
index 000000000..bcf023b31
--- /dev/null
+++ b/changelogs/unreleased/gh-6485-bug-of-decimal.md
@@ -0,0 +1,8 @@
+## bugfix/sql
+
+* Fixed truncation of DECIMAL during implicit cast to INTEGER in LIMIT and
+  OFFSET.
+* Fixed truncation of DECIMAL during implicit cast to INTEGER when value is used
+  in an index.
+* Fixed assert on cast of DECIMAL value that greater than -1.0 and less than 0.0
+  to INTEGER (gh-6485).
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index bc6f9b35a..7e94dc8bb 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -1338,7 +1338,7 @@ mem_to_int_precise(struct Mem *mem)
 	if (mem->type == MEM_TYPE_DOUBLE)
 		return double_to_int_precise(mem);
 	if (mem->type == MEM_TYPE_DEC)
-		return dec_to_int(mem);
+		return dec_to_int_precise(mem);
 	return -1;
 }
 
diff --git a/test/sql-tap/gh-6485-bugs-in-decimal.test.lua b/test/sql-tap/gh-6485-bugs-in-decimal.test.lua
index 0b9b2ea0a..fc2ac963d 100755
--- a/test/sql-tap/gh-6485-bugs-in-decimal.test.lua
+++ b/test/sql-tap/gh-6485-bugs-in-decimal.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(3)
+test:plan(5)
 
 -- Make sure DECIMAL is not truncated when used in an index.
 test:do_execsql_test(
@@ -35,4 +35,23 @@ test:do_execsql_test(
     ]], {
     })
 
+-- Make sure DECIMAL is not truncated when used in LIMIT and OFFSET.
+test:do_catchsql_test(
+    "gh-6485-4",
+    [[
+        SELECT 1 LIMIT CAST(1.5 AS DECIMAL);
+    ]], {
+        1, [[Failed to execute SQL statement: ]]..
+        [[Only positive integers are allowed in the LIMIT clause]]
+    })
+
+test:do_catchsql_test(
+    "gh-6485-5",
+    [[
+        SELECT 1 LIMIT 1 OFFSET CAST(1.5 AS DECIMAL);
+    ]], {
+        1, [[Failed to execute SQL statement: ]]..
+        [[Only positive integers are allowed in the OFFSET clause]]
+    })
+
 test:finish_test()
-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL
  2021-10-04 13:30 [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL Mergen Imeev via Tarantool-patches
@ 2021-10-11 21:58 ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 0 replies; 6+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-10-11 21:58 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

Thanks for the patchset!

LGTM.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL
@ 2021-10-04 13:30 Mergen Imeev via Tarantool-patches
  2021-10-11 21:58 ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 1 reply; 6+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-10-04 13:30 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

This patch-set fixes three bugs with DECIMAL.

https://github.com/tarantool/tarantool/issues/6485
https://github.com/tarantool/tarantool/tree/imeevma/gh-6485-fix-bugs-with-decimal

Mergen Imeev (3):
  sql: fix truncation of DECIMAL in implicit cast
  sql: fix cast of small negative DECIMAL to INTEGER
  sql: do not truncate DECIMAL in LIMIT and OFFSET

 .../unreleased/gh-6485-bug-of-decimal.md      |  8 +++
 src/box/sql/mem.c                             | 16 +++---
 test/sql-tap/gh-6485-bugs-in-decimal.test.lua | 57 +++++++++++++++++++
 3 files changed, 74 insertions(+), 7 deletions(-)
 create mode 100644 changelogs/unreleased/gh-6485-bug-of-decimal.md
 create mode 100755 test/sql-tap/gh-6485-bugs-in-decimal.test.lua

-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-19  6:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-19  6:19 [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL Mergen Imeev via Tarantool-patches
2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 1/3] sql: fix truncation of DECIMAL in implicit cast Mergen Imeev via Tarantool-patches
2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 2/3] sql: fix cast of small negative DECIMAL to INTEGER Mergen Imeev via Tarantool-patches
2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 3/3] sql: do not truncate DECIMAL in LIMIT and OFFSET Mergen Imeev via Tarantool-patches
  -- strict thread matches above, loose matches on Subject: below --
2021-10-04 13:30 [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL Mergen Imeev via Tarantool-patches
2021-10-11 21:58 ` Vladislav Shpilevoy via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox