Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf()
@ 2021-11-15 16:06 Mergen Imeev via Tarantool-patches
  2021-11-15 16:06 ` [Tarantool-patches] [PATCH v1 1/2] sql: omit quotes for UUID values in errors Mergen Imeev via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-11-15 16:06 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

This patch-set introduces function mem_snprintf() which can be used to get
string representation of a MEM.

https://github.com/tarantool/tarantool/issues/4145
https://github.com/tarantool/tarantool/tree/imeevma/gh-4145-follow-up

Mergen Imeev (2):
  sql: omit quotes for UUID values in errors
  sql: introduce mem_snprintf()

 src/box/sql/func.c                            |  9 ++-
 src/box/sql/mem.c                             | 75 ++++++++++---------
 src/box/sql/mem.h                             |  8 ++
 src/box/sql/printf.c                          | 15 +---
 test/sql-tap/cast.test.lua                    |  4 +-
 test/sql-tap/decimal.test.lua                 |  6 +-
 .../gh-5364-define-bit-wise-rules.test.lua    |  8 +-
 ...-5756-implicit-cast-in-arithmetic.test.lua | 10 +--
 test/sql-tap/sql-errors.test.lua              |  7 +-
 test/sql-tap/uuid.test.lua                    | 58 +++++++-------
 test/sql/types.result                         |  2 +-
 11 files changed, 108 insertions(+), 94 deletions(-)

-- 
2.25.1


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

* [Tarantool-patches] [PATCH v1 1/2] sql: omit quotes for UUID values in errors
  2021-11-15 16:06 [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf() Mergen Imeev via Tarantool-patches
@ 2021-11-15 16:06 ` Mergen Imeev via Tarantool-patches
  2021-11-15 16:06 ` [Tarantool-patches] [PATCH v1 2/2] sql: introduce mem_snprintf() Mergen Imeev via Tarantool-patches
  2021-11-21 15:27 ` [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf() Vladislav Shpilevoy via Tarantool-patches
  2 siblings, 0 replies; 8+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-11-15 16:06 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

This patch removes quotes from the representation of UUID values in the
error description. This is because UUID values are printed without
quotes elsewhere.
---
 src/box/sql/mem.c                             |  2 +-
 test/sql-tap/cast.test.lua                    |  4 +-
 test/sql-tap/decimal.test.lua                 |  6 +-
 .../gh-5364-define-bit-wise-rules.test.lua    |  8 +--
 ...-5756-implicit-cast-in-arithmetic.test.lua | 10 ++--
 test/sql-tap/sql-errors.test.lua              |  2 +-
 test/sql-tap/uuid.test.lua                    | 58 +++++++++----------
 7 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 7ea68f868..9ddeea5bb 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -163,7 +163,7 @@ mem_str(const struct Mem *mem)
 	}
 	case MEM_TYPE_UUID:
 		tt_uuid_to_string(&mem->u.uuid, buf);
-		return tt_sprintf("%s('%s')", type, buf);
+		return tt_sprintf("%s(%s)", type, buf);
 	case MEM_TYPE_BOOL:
 		return tt_sprintf("%s(%s)", type, mem->u.b ? "TRUE" : "FALSE");
 	default:
diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index d5c9cbbc6..5861799b3 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -1067,7 +1067,7 @@ test:do_catchsql_test(
         INSERT INTO t3(s) VALUES(]]..uuid..[[);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to string"
+           "uuid(11111111-1111-1111-1111-111111111111) to string"
     })
 
 -- Make sure that UUID cannot be implicitly cast to VARBINARY.
@@ -1077,7 +1077,7 @@ test:do_catchsql_test(
         INSERT INTO t4(v) VALUES(]]..uuid..[[);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to varbinary"
+           "uuid(11111111-1111-1111-1111-111111111111) to varbinary"
     })
 
 -- Make sure that STRING and VARBINARY cannot be implicitly cast to UUID.
diff --git a/test/sql-tap/decimal.test.lua b/test/sql-tap/decimal.test.lua
index a6d66b75b..ab211ba81 100755
--- a/test/sql-tap/decimal.test.lua
+++ b/test/sql-tap/decimal.test.lua
@@ -336,7 +336,7 @@ test:do_catchsql_test(
     [[
         SELECT u > CAST('11111111-1111-1111-1111-111111111111' AS UUID) FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to number"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to number"
     })
 
 test:do_catchsql_test(
@@ -392,7 +392,7 @@ test:do_catchsql_test(
     [[
         SELECT u = CAST('11111111-1111-1111-1111-111111111111' AS UUID) FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to number"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to number"
     })
 
 test:do_catchsql_test(
@@ -562,7 +562,7 @@ test:do_catchsql_test(
     [[
         SELECT cast(cast(x'11111111111111111111111111111111' AS UUID) AS DECIMAL);
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to decimal"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to decimal"
     })
 
 test:execsql([[
diff --git a/test/sql-tap/gh-5364-define-bit-wise-rules.test.lua b/test/sql-tap/gh-5364-define-bit-wise-rules.test.lua
index 2a6a60a81..e1ab8abe8 100755
--- a/test/sql-tap/gh-5364-define-bit-wise-rules.test.lua
+++ b/test/sql-tap/gh-5364-define-bit-wise-rules.test.lua
@@ -60,7 +60,7 @@ test:do_catchsql_test(
         SELECT 9 >> CAST('11111111-1111-1111-1111-111111111111' AS UUID);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+           "uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 test:do_execsql_test(
@@ -117,7 +117,7 @@ test:do_catchsql_test(
         SELECT 9 << CAST('11111111-1111-1111-1111-111111111111' AS UUID);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+           "uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 test:do_execsql_test(
@@ -174,7 +174,7 @@ test:do_catchsql_test(
         SELECT 9 & CAST('11111111-1111-1111-1111-111111111111' AS UUID);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+           "uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 test:do_execsql_test(
@@ -231,7 +231,7 @@ test:do_catchsql_test(
         SELECT 9 | CAST('11111111-1111-1111-1111-111111111111' AS UUID);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+           "uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 test:finish_test()
diff --git a/test/sql-tap/gh-5756-implicit-cast-in-arithmetic.test.lua b/test/sql-tap/gh-5756-implicit-cast-in-arithmetic.test.lua
index bf636a261..46bac159b 100755
--- a/test/sql-tap/gh-5756-implicit-cast-in-arithmetic.test.lua
+++ b/test/sql-tap/gh-5756-implicit-cast-in-arithmetic.test.lua
@@ -60,7 +60,7 @@ test:do_catchsql_test(
         SELECT 9 + CAST('11111111-1111-1111-1111-111111111111' AS UUID);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to integer, decimal or double"
+           "uuid(11111111-1111-1111-1111-111111111111) to integer, decimal or double"
     })
 
 test:do_execsql_test(
@@ -117,7 +117,7 @@ test:do_catchsql_test(
         SELECT 9 - CAST('11111111-1111-1111-1111-111111111111' AS UUID);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to integer, decimal or double"
+           "uuid(11111111-1111-1111-1111-111111111111) to integer, decimal or double"
     })
 
 test:do_execsql_test(
@@ -174,7 +174,7 @@ test:do_catchsql_test(
         SELECT 9 * CAST('11111111-1111-1111-1111-111111111111' AS UUID);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to integer, decimal or double"
+           "uuid(11111111-1111-1111-1111-111111111111) to integer, decimal or double"
     })
 
 test:do_execsql_test(
@@ -231,7 +231,7 @@ test:do_catchsql_test(
         SELECT 9 / CAST('11111111-1111-1111-1111-111111111111' AS UUID);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to integer, decimal or double"
+           "uuid(11111111-1111-1111-1111-111111111111) to integer, decimal or double"
     })
 
 test:do_execsql_test(
@@ -288,7 +288,7 @@ test:do_catchsql_test(
         SELECT 9 % CAST('11111111-1111-1111-1111-111111111111' AS UUID);
     ]], {
         1, "Type mismatch: can not convert "..
-           "uuid('11111111-1111-1111-1111-111111111111') to integer"
+           "uuid(11111111-1111-1111-1111-111111111111) to integer"
     })
 
 test:finish_test()
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index 08a675101..a9aa5acf7 100755
--- a/test/sql-tap/sql-errors.test.lua
+++ b/test/sql-tap/sql-errors.test.lua
@@ -850,7 +850,7 @@ test:do_catchsql_test(
 	[[
 		SELECT CAST(CAST('11111111-1111-1111-1111-111111111111' AS UUID) AS UNSIGNED);
 	]], {
-		1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+		1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to unsigned"
 	})
 
 local bin = ''
diff --git a/test/sql-tap/uuid.test.lua b/test/sql-tap/uuid.test.lua
index 884b1daf9..8421e50c7 100755
--- a/test/sql-tap/uuid.test.lua
+++ b/test/sql-tap/uuid.test.lua
@@ -493,7 +493,7 @@ test:do_catchsql_test(
     [[
         SELECT u || u from t2;
     ]], {
-        1, "Inconsistent types: expected string or varbinary got uuid('11111111-1111-1111-1111-111111111111')"
+        1, "Inconsistent types: expected string or varbinary got uuid(11111111-1111-1111-1111-111111111111)"
     })
 
 local func = {language = 'Lua', body = 'function(x) return type(x) end',
@@ -552,7 +552,7 @@ test:do_catchsql_test(
     [[
         SELECT cast(u AS UNSIGNED) FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 test:do_execsql_test(
@@ -570,7 +570,7 @@ test:do_catchsql_test(
     [[
         SELECT cast(u AS NUMBER) FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to number"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to number"
     })
 
 test:do_catchsql_test(
@@ -578,7 +578,7 @@ test:do_catchsql_test(
     [[
         SELECT cast(u AS DOUBLE) FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to double"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to double"
     })
 
 test:do_catchsql_test(
@@ -586,7 +586,7 @@ test:do_catchsql_test(
     [[
         SELECT cast(u AS INTEGER) FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to integer"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to integer"
     })
 
 test:do_catchsql_test(
@@ -594,7 +594,7 @@ test:do_catchsql_test(
     [[
         SELECT cast(u AS BOOLEAN) FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to boolean"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to boolean"
     })
 
 test:do_execsql_test(
@@ -707,7 +707,7 @@ test:do_catchsql_test(
     [[
         INSERT INTO tu(u) SELECT u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 test:do_catchsql_test(
@@ -715,7 +715,7 @@ test:do_catchsql_test(
     [[
         INSERT INTO ts(s) SELECT u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to string"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to string"
     })
 
 test:do_catchsql_test(
@@ -723,7 +723,7 @@ test:do_catchsql_test(
     [[
         INSERT INTO tn(n) SELECT u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to number"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to number"
     })
 
 test:do_catchsql_test(
@@ -731,7 +731,7 @@ test:do_catchsql_test(
     [[
         INSERT INTO td(d) SELECT u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to double"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to double"
     })
 
 test:do_catchsql_test(
@@ -739,7 +739,7 @@ test:do_catchsql_test(
     [[
         INSERT INTO ti(i) SELECT u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to integer"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to integer"
     })
 
 test:do_catchsql_test(
@@ -747,7 +747,7 @@ test:do_catchsql_test(
     [[
         INSERT INTO tb(b) SELECT u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to boolean"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to boolean"
     })
 
 test:do_catchsql_test(
@@ -755,7 +755,7 @@ test:do_catchsql_test(
     [[
         INSERT INTO tv(v) SELECT u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to varbinary"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to varbinary"
     })
 
 test:do_execsql_test(
@@ -945,7 +945,7 @@ test:do_catchsql_test(
     [[
         SELECT -u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to integer, decimal or double"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to integer, decimal or double"
     })
 
 test:do_catchsql_test(
@@ -953,7 +953,7 @@ test:do_catchsql_test(
     [[
         SELECT u + 1 FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to integer, decimal or double"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to integer, decimal or double"
     })
 
 test:do_catchsql_test(
@@ -961,7 +961,7 @@ test:do_catchsql_test(
     [[
         SELECT u - 1 FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to integer, decimal or double"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to integer, decimal or double"
     })
 
 test:do_catchsql_test(
@@ -969,7 +969,7 @@ test:do_catchsql_test(
     [[
         SELECT u * 1 FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to integer, decimal or double"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to integer, decimal or double"
     })
 
 test:do_catchsql_test(
@@ -977,7 +977,7 @@ test:do_catchsql_test(
     [[
         SELECT u / 1 FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to integer, decimal or double"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to integer, decimal or double"
     })
 
 test:do_catchsql_test(
@@ -985,7 +985,7 @@ test:do_catchsql_test(
     [[
         SELECT u % 1 FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to integer"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to integer"
     })
 
 -- Check that bitwise operations work with UUIDs as intended.
@@ -994,7 +994,7 @@ test:do_catchsql_test(
     [[
         SELECT ~u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 test:do_catchsql_test(
@@ -1002,7 +1002,7 @@ test:do_catchsql_test(
     [[
         SELECT u >> 1 FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 test:do_catchsql_test(
@@ -1010,7 +1010,7 @@ test:do_catchsql_test(
     [[
         SELECT u << 1 FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 test:do_catchsql_test(
@@ -1018,7 +1018,7 @@ test:do_catchsql_test(
     [[
         SELECT u | 1 FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 test:do_catchsql_test(
@@ -1026,7 +1026,7 @@ test:do_catchsql_test(
     [[
         SELECT u & 1 FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to unsigned"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to unsigned"
     })
 
 -- Check that logical operations work with UUIDs as intended.
@@ -1035,7 +1035,7 @@ test:do_catchsql_test(
     [[
         SELECT NOT u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to boolean"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to boolean"
     })
 
 test:do_catchsql_test(
@@ -1043,7 +1043,7 @@ test:do_catchsql_test(
     [[
         SELECT u AND true FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to boolean"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to boolean"
     })
 
 test:do_catchsql_test(
@@ -1051,7 +1051,7 @@ test:do_catchsql_test(
     [[
         SELECT u OR true FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to boolean"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to boolean"
     })
 
 test:do_catchsql_test(
@@ -1059,7 +1059,7 @@ test:do_catchsql_test(
     [[
         SELECT true AND u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to boolean"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to boolean"
     })
 
 test:do_catchsql_test(
@@ -1067,7 +1067,7 @@ test:do_catchsql_test(
     [[
         SELECT true OR u FROM t2;
     ]], {
-        1, "Type mismatch: can not convert uuid('11111111-1111-1111-1111-111111111111') to boolean"
+        1, "Type mismatch: can not convert uuid(11111111-1111-1111-1111-111111111111) to boolean"
     })
 
 -- Check that comparison with UUID works as intended.
-- 
2.25.1


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

* [Tarantool-patches] [PATCH v1 2/2] sql: introduce mem_snprintf()
  2021-11-15 16:06 [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf() Mergen Imeev via Tarantool-patches
  2021-11-15 16:06 ` [Tarantool-patches] [PATCH v1 1/2] sql: omit quotes for UUID values in errors Mergen Imeev via Tarantool-patches
@ 2021-11-15 16:06 ` Mergen Imeev via Tarantool-patches
  2021-11-17 23:03   ` Vladislav Shpilevoy via Tarantool-patches
  2021-11-21 15:27 ` [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf() Vladislav Shpilevoy via Tarantool-patches
  2 siblings, 1 reply; 8+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-11-15 16:06 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

This patch introduces the mem_snprintf() function, which writes the
string representation of a MEM to buf.
---
 src/box/sql/func.c               |  9 +++-
 src/box/sql/mem.c                | 73 +++++++++++++++++---------------
 src/box/sql/mem.h                |  8 ++++
 src/box/sql/printf.c             | 15 ++-----
 test/sql-tap/sql-errors.test.lua |  5 ++-
 test/sql/types.result            |  2 +-
 6 files changed, 63 insertions(+), 49 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 5abaf490d..0f8f0b5cc 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -869,9 +869,14 @@ func_printf(struct sql_context *ctx, int argc, const struct Mem *argv)
 	if (argc < 1 || mem_is_null(&argv[0]))
 		return;
 	if (argc == 1 || !mem_is_str(&argv[0])) {
-		struct Mem *mem = ctx->pOut;
-		if (mem_copy(mem, &argv[0]) != 0 || mem_to_str(mem) != 0)
+		uint32_t size = mem_snprintf(NULL, 0, &argv[0]);
+		char *str = sqlDbMallocRawNN(sql_get(), size + 1);
+		if (str == NULL) {
 			ctx->is_aborted = true;
+			return;
+		}
+		mem_snprintf(str, size + 1, &argv[0]);
+		mem_set_str_allocated(ctx->pOut, str, size);
 		return;
 	}
 	struct PrintfArguments pargs;
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 9ddeea5bb..6e1729f32 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -116,30 +116,51 @@ mem_is_field_compatible(const struct Mem *mem, enum field_type type)
 	return field_mp_plain_type_is_compatible(type, mp_type, true);
 }
 
-const char *
-mem_str(const struct Mem *mem)
+int
+mem_snprintf(char *buf, uint32_t size, const struct Mem *mem)
 {
-	char buf[STR_VALUE_MAX_LEN];
-	const char *type = mem_type_to_str(mem);
 	switch (mem->type) {
 	case MEM_TYPE_NULL:
-		return "NULL";
+		return snprintf(buf, size, "NULL");
 	case MEM_TYPE_STR:
+	case MEM_TYPE_BIN:
+		return snprintf(buf, size, "%.*s", mem->n, mem->z);
+	case MEM_TYPE_INT:
+		return snprintf(buf, size, "%lld", mem->u.i);
+	case MEM_TYPE_UINT:
+		return snprintf(buf, size, "%llu",
+				(unsigned long long)mem->u.u);
+	case MEM_TYPE_DOUBLE: {
+		char str[BUF_SIZE];
+		sql_snprintf(BUF_SIZE, str, "%!.15g", mem->u.r);
+		return snprintf(buf, size, "%s", str);
+	}
+	case MEM_TYPE_DEC:
+		return snprintf(buf, size, "%s", decimal_str(&mem->u.d));
+	case MEM_TYPE_MAP:
+	case MEM_TYPE_ARRAY:
+		return mp_snprint(buf, size, mem->z);
+	case MEM_TYPE_UUID:
+		return snprintf(buf, size, "%s", tt_uuid_str(&mem->u.uuid));
+	case MEM_TYPE_BOOL:
+		return snprintf(buf, size, "%s", mem->u.b ? "TRUE" : "FALSE");
+	default:
+		return snprintf(buf, size, "unknown");
+	}
+}
+
+const char *
+mem_str(const struct Mem *mem)
+{
+	const char *type = mem_type_to_str(mem);
+	if (mem->type == MEM_TYPE_STR) {
 		if (mem->n <= STR_VALUE_MAX_LEN)
 			return tt_sprintf("%s('%.*s')", type, mem->n, mem->z);
 		return tt_sprintf("%s('%.*s...)", type, STR_VALUE_MAX_LEN,
 				  mem->z);
-	case MEM_TYPE_INT:
-		return tt_sprintf("%s(%lld)", type, mem->u.i);
-	case MEM_TYPE_UINT:
-		return tt_sprintf("%s(%llu)", type, mem->u.u);
-	case MEM_TYPE_DOUBLE:
-		sql_snprintf(STR_VALUE_MAX_LEN, buf, "%!.15g", mem->u.r);
-		return tt_sprintf("%s(%s)", type, buf);
-	case MEM_TYPE_DEC:
-		decimal_to_string(&mem->u.d, buf);
-		return tt_sprintf("%s(%s)", type, buf);
-	case MEM_TYPE_BIN: {
+	}
+	char buf[STR_VALUE_MAX_LEN];
+	if (mem->type == MEM_TYPE_BIN) {
 		int len = MIN(mem->n, STR_VALUE_MAX_LEN / 2);
 		for (int i = 0; i < len; ++i) {
 			int n = (mem->z[i] & 0xF0) >> 4;
@@ -151,24 +172,10 @@ mem_str(const struct Mem *mem)
 			return tt_sprintf("%s(x'%.*s...)", type, len * 2, buf);
 		return tt_sprintf("%s(x'%.*s')", type, len * 2, buf);
 	}
-	case MEM_TYPE_MAP:
-	case MEM_TYPE_ARRAY: {
-		const char *str = mp_str(mem->z);
-		uint32_t len = strlen(str);
-		uint32_t minlen = MIN(STR_VALUE_MAX_LEN, len);
-		memcpy(buf, str, minlen);
-		if (len <= STR_VALUE_MAX_LEN)
-			return tt_sprintf("%s(%.*s)", type, minlen, buf);
-		return tt_sprintf("%s(%.*s...)", type, minlen, buf);
-	}
-	case MEM_TYPE_UUID:
-		tt_uuid_to_string(&mem->u.uuid, buf);
+	int size = mem_snprintf(buf, STR_VALUE_MAX_LEN, mem);
+	if (size <= STR_VALUE_MAX_LEN)
 		return tt_sprintf("%s(%s)", type, buf);
-	case MEM_TYPE_BOOL:
-		return tt_sprintf("%s(%s)", type, mem->u.b ? "TRUE" : "FALSE");
-	default:
-		return "unknown";
-	}
+	return tt_sprintf("%s(%.*s...)", type, STR_VALUE_MAX_LEN, buf);
 }
 
 static const char *
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index e3313ff98..62830c3f5 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -246,6 +246,14 @@ mem_is_any_null(const struct Mem *mem1, const struct Mem *mem2)
 bool
 mem_is_field_compatible(const struct Mem *mem, enum field_type type);
 
+/**
+ * Write a NULL-terminated string representation of a MEM to buf. Returns the
+ * number of bytes required to write the value, excluding '\0'. If the return
+ * value is equal to or greater than size, then the value has been truncated.
+ */
+int
+mem_snprintf(char *buf, uint32_t size, const struct Mem *mem);
+
 /**
  * Return a string that contains description of type and value of MEM. String is
  * either allocated using static_alloc() of just a static variable. This
diff --git a/src/box/sql/printf.c b/src/box/sql/printf.c
index 8da7c9878..08e18c15d 100644
--- a/src/box/sql/printf.c
+++ b/src/box/sql/printf.c
@@ -160,19 +160,12 @@ getTextArg(PrintfArguments * p)
 {
 	if (p->nArg <= p->nUsed)
 		return 0;
-	struct Mem mem;
-	mem_create(&mem);
-	mem_copy_as_ephemeral(&mem, &p->apArg[p->nUsed++]);
-	if (mem_to_str(&mem) != 0) {
-		mem_destroy(&mem);
-		return NULL;
-	}
-	char *str = sqlDbMallocRawNN(sql_get(), mem.n + 1);
+	const struct Mem *mem = &p->apArg[p->nUsed++];
+	uint32_t size = mem_snprintf(NULL, 0, mem);
+	char *str = sqlDbMallocRawNN(sql_get(), size + 1);
 	if (str == NULL)
 		return NULL;
-	memcpy(str, mem.z, mem.n);
-	str[mem.n] = '\0';
-	mem_destroy(&mem);
+	mem_snprintf(str, size + 1, mem);
 	return str;
 }
 
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index a9aa5acf7..6cecd7c6d 100755
--- a/test/sql-tap/sql-errors.test.lua
+++ b/test/sql-tap/sql-errors.test.lua
@@ -812,7 +812,8 @@ test:do_catchsql_test(
 	"SELECT CAST(a AS UNSIGNED) from test;", {
 		1, 'Type mismatch: can not convert array(["aaaaaaaaaaaaaaaaaa'..
 		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'..
-		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...) to unsigned'
+		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...) to '..
+		'unsigned'
 	})
 
 test:do_catchsql_test(
@@ -820,7 +821,7 @@ test:do_catchsql_test(
 	"SELECT CAST(m AS UNSIGNED) from test;", {
 		1, 'Type mismatch: can not convert map({"a": 1, "b": "aaaaaaa'..
 		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'..
-		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...) to unsigned'
+		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...) to unsigned'
 	})
 
 test:execsql('DROP TABLE test;')
diff --git a/test/sql/types.result b/test/sql/types.result
index 947795b03..5a822fddf 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -1623,7 +1623,7 @@ box.execute('INSERT INTO t1(a) SELECT a FROM t2;')
 - null
 - 'Type mismatch: can not convert array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13,
   14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33,
-  34, ...) to scalar'
+  34,...) to scalar'
 ...
 s:drop()
 ---
-- 
2.25.1


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

* Re: [Tarantool-patches] [PATCH v1 2/2] sql: introduce mem_snprintf()
  2021-11-15 16:06 ` [Tarantool-patches] [PATCH v1 2/2] sql: introduce mem_snprintf() Mergen Imeev via Tarantool-patches
@ 2021-11-17 23:03   ` Vladislav Shpilevoy via Tarantool-patches
  2021-11-19 12:02     ` Mergen Imeev via Tarantool-patches
  0 siblings, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-11-17 23:03 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

Hi! Thanks for the patch!

See 5 comments below.

> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index 5abaf490d..0f8f0b5cc 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -869,9 +869,14 @@ func_printf(struct sql_context *ctx, int argc, const struct Mem *argv)
>  	if (argc < 1 || mem_is_null(&argv[0]))
>  		return;
>  	if (argc == 1 || !mem_is_str(&argv[0])) {
> -		struct Mem *mem = ctx->pOut;
> -		if (mem_copy(mem, &argv[0]) != 0 || mem_to_str(mem) != 0)
> +		uint32_t size = mem_snprintf(NULL, 0, &argv[0]);

1. You are not checking for an error here. mem_snprintf() returns int,
not uint. I am fine with making it never failing, but then we need
to at least add some assertions inside of mem_snprintf() that its
internal snprintf() calls never fail. We can keep returning 'int' though
because it makes the function compatible with SNPRINT macro.

> +		char *str = sqlDbMallocRawNN(sql_get(), size + 1);
> +		if (str == NULL) {
>  			ctx->is_aborted = true;
> +			return;
> +		}
> +		mem_snprintf(str, size + 1, &argv[0]);
> +		mem_set_str_allocated(ctx->pOut, str, size);
>  		return;
>  	}
> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index 9ddeea5bb..6e1729f32 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -116,30 +116,51 @@ mem_is_field_compatible(const struct Mem *mem, enum field_type type)
>  	return field_mp_plain_type_is_compatible(type, mp_type, true);
>  }
>  
> -const char *
> -mem_str(const struct Mem *mem)
> +int
> +mem_snprintf(char *buf, uint32_t size, const struct Mem *mem)
>  {
> -	char buf[STR_VALUE_MAX_LEN];
> -	const char *type = mem_type_to_str(mem);
>  	switch (mem->type) {
>  	case MEM_TYPE_NULL:
> -		return "NULL";
> +		return snprintf(buf, size, "NULL");
>  	case MEM_TYPE_STR:
> +	case MEM_TYPE_BIN:
> +		return snprintf(buf, size, "%.*s", mem->n, mem->z);

2. Why is bin displayed as hex in mem_str() but as a plain string here?

> +	case MEM_TYPE_INT:
> +		return snprintf(buf, size, "%lld", mem->u.i);
> +	case MEM_TYPE_UINT:
> +		return snprintf(buf, size, "%llu",
> +				(unsigned long long)mem->u.u);
> +	case MEM_TYPE_DOUBLE: {
> +		char str[BUF_SIZE];
> +		sql_snprintf(BUF_SIZE, str, "%!.15g", mem->u.r);
> +		return snprintf(buf, size, "%s", str);
> +	}
> +	case MEM_TYPE_DEC:
> +		return snprintf(buf, size, "%s", decimal_str(&mem->u.d));
> +	case MEM_TYPE_MAP:
> +	case MEM_TYPE_ARRAY:
> +		return mp_snprint(buf, size, mem->z);
> +	case MEM_TYPE_UUID:
> +		return snprintf(buf, size, "%s", tt_uuid_str(&mem->u.uuid));
> +	case MEM_TYPE_BOOL:
> +		return snprintf(buf, size, "%s", mem->u.b ? "TRUE" : "FALSE");

3. You can drop '%s' here:

	return snprintf(buf, size, mem->u.b ? "TRUE" : "FALSE");

> +	default:
> +		return snprintf(buf, size, "unknown");

4. Maybe add an assertion that it is not possible? We don't have non-typed
mems AFAIK.

> +	}
> +}
> diff --git a/src/box/sql/printf.c b/src/box/sql/printf.c
> index 8da7c9878..08e18c15d 100644
> --- a/src/box/sql/printf.c
> +++ b/src/box/sql/printf.c
> @@ -160,19 +160,12 @@ getTextArg(PrintfArguments * p)
>  {
>  	if (p->nArg <= p->nUsed)
>  		return 0;
> -	struct Mem mem;
> -	mem_create(&mem);
> -	mem_copy_as_ephemeral(&mem, &p->apArg[p->nUsed++]);
> -	if (mem_to_str(&mem) != 0) {
> -		mem_destroy(&mem);
> -		return NULL;
> -	}
> -	char *str = sqlDbMallocRawNN(sql_get(), mem.n + 1);
> +	const struct Mem *mem = &p->apArg[p->nUsed++];
> +	uint32_t size = mem_snprintf(NULL, 0, mem);
> +	char *str = sqlDbMallocRawNN(sql_get(), size + 1);
>  	if (str == NULL)
>  		return NULL;
> -	memcpy(str, mem.z, mem.n);
> -	str[mem.n] = '\0';
> -	mem_destroy(&mem);
> +	mem_snprintf(str, size + 1, mem);

5. Hm. I suspect this pattern of snprintf + malloc + snprintf
is going to be frequent. Maybe wrap it into a function like
mem_strdup()? It would do these 3 actions inside. WDYT?

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

* Re: [Tarantool-patches] [PATCH v1 2/2] sql: introduce mem_snprintf()
  2021-11-17 23:03   ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-11-19 12:02     ` Mergen Imeev via Tarantool-patches
  0 siblings, 0 replies; 8+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-11-19 12:02 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Thank you for the review! My answers, diff and new patch below. Also, I found
out that mem_str() for MEM with NULL returns "NULL(NULL)". Fixed, see diff.

On Thu, Nov 18, 2021 at 12:03:18AM +0100, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> See 5 comments below.
> 
> > diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> > index 5abaf490d..0f8f0b5cc 100644
> > --- a/src/box/sql/func.c
> > +++ b/src/box/sql/func.c
> > @@ -869,9 +869,14 @@ func_printf(struct sql_context *ctx, int argc, const struct Mem *argv)
> >  	if (argc < 1 || mem_is_null(&argv[0]))
> >  		return;
> >  	if (argc == 1 || !mem_is_str(&argv[0])) {
> > -		struct Mem *mem = ctx->pOut;
> > -		if (mem_copy(mem, &argv[0]) != 0 || mem_to_str(mem) != 0)
> > +		uint32_t size = mem_snprintf(NULL, 0, &argv[0]);
> 
> 1. You are not checking for an error here. mem_snprintf() returns int,
> not uint. I am fine with making it never failing, but then we need
> to at least add some assertions inside of mem_snprintf() that its
> internal snprintf() calls never fail. We can keep returning 'int' though
> because it makes the function compatible with SNPRINT macro.
> 
Thanks. I added two asserts in mem_snprintf(), one of which checks that returned
by snprintf() value is non-negative.

> > +		char *str = sqlDbMallocRawNN(sql_get(), size + 1);
> > +		if (str == NULL) {
> >  			ctx->is_aborted = true;
> > +			return;
> > +		}
> > +		mem_snprintf(str, size + 1, &argv[0]);
> > +		mem_set_str_allocated(ctx->pOut, str, size);
> >  		return;
> >  	}
> > diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> > index 9ddeea5bb..6e1729f32 100644
> > --- a/src/box/sql/mem.c
> > +++ b/src/box/sql/mem.c
> > @@ -116,30 +116,51 @@ mem_is_field_compatible(const struct Mem *mem, enum field_type type)
> >  	return field_mp_plain_type_is_compatible(type, mp_type, true);
> >  }
> >  
> > -const char *
> > -mem_str(const struct Mem *mem)
> > +int
> > +mem_snprintf(char *buf, uint32_t size, const struct Mem *mem)
> >  {
> > -	char buf[STR_VALUE_MAX_LEN];
> > -	const char *type = mem_type_to_str(mem);
> >  	switch (mem->type) {
> >  	case MEM_TYPE_NULL:
> > -		return "NULL";
> > +		return snprintf(buf, size, "NULL");
> >  	case MEM_TYPE_STR:
> > +	case MEM_TYPE_BIN:
> > +		return snprintf(buf, size, "%.*s", mem->n, mem->z);
> 
> 2. Why is bin displayed as hex in mem_str() but as a plain string here?
> 
I decided not to break compatibility with the old result of the built-in
printf() function. As for mem_str() - we have to get the readable value there,
so we can't just print the original value. This does not work for STRING values,
as we might end up with an unreadable value there.

> > +	case MEM_TYPE_INT:
> > +		return snprintf(buf, size, "%lld", mem->u.i);
> > +	case MEM_TYPE_UINT:
> > +		return snprintf(buf, size, "%llu",
> > +				(unsigned long long)mem->u.u);
> > +	case MEM_TYPE_DOUBLE: {
> > +		char str[BUF_SIZE];
> > +		sql_snprintf(BUF_SIZE, str, "%!.15g", mem->u.r);
> > +		return snprintf(buf, size, "%s", str);
> > +	}
> > +	case MEM_TYPE_DEC:
> > +		return snprintf(buf, size, "%s", decimal_str(&mem->u.d));
> > +	case MEM_TYPE_MAP:
> > +	case MEM_TYPE_ARRAY:
> > +		return mp_snprint(buf, size, mem->z);
> > +	case MEM_TYPE_UUID:
> > +		return snprintf(buf, size, "%s", tt_uuid_str(&mem->u.uuid));
> > +	case MEM_TYPE_BOOL:
> > +		return snprintf(buf, size, "%s", mem->u.b ? "TRUE" : "FALSE");
> 
> 3. You can drop '%s' here:
> 
> 	return snprintf(buf, size, mem->u.b ? "TRUE" : "FALSE");
> 
True, fixed.

> > +	default:
> > +		return snprintf(buf, size, "unknown");
> 
> 4. Maybe add an assertion that it is not possible? We don't have non-typed
> mems AFAIK.
> 
Added.

> > +	}
> > +}
> > diff --git a/src/box/sql/printf.c b/src/box/sql/printf.c
> > index 8da7c9878..08e18c15d 100644
> > --- a/src/box/sql/printf.c
> > +++ b/src/box/sql/printf.c
> > @@ -160,19 +160,12 @@ getTextArg(PrintfArguments * p)
> >  {
> >  	if (p->nArg <= p->nUsed)
> >  		return 0;
> > -	struct Mem mem;
> > -	mem_create(&mem);
> > -	mem_copy_as_ephemeral(&mem, &p->apArg[p->nUsed++]);
> > -	if (mem_to_str(&mem) != 0) {
> > -		mem_destroy(&mem);
> > -		return NULL;
> > -	}
> > -	char *str = sqlDbMallocRawNN(sql_get(), mem.n + 1);
> > +	const struct Mem *mem = &p->apArg[p->nUsed++];
> > +	uint32_t size = mem_snprintf(NULL, 0, mem);
> > +	char *str = sqlDbMallocRawNN(sql_get(), size + 1);
> >  	if (str == NULL)
> >  		return NULL;
> > -	memcpy(str, mem.z, mem.n);
> > -	str[mem.n] = '\0';
> > -	mem_destroy(&mem);
> > +	mem_snprintf(str, size + 1, mem);
> 
> 5. Hm. I suspect this pattern of snprintf + malloc + snprintf
> is going to be frequent. Maybe wrap it into a function like
> mem_strdup()? It would do these 3 actions inside. WDYT?
I agree. I added mem_strdup(). I decided not to make mem_snprintf() static,
since it was designed as one of the functions for working with MEM outside
mem.c.

Diff:

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 0f8f0b5cc..26ccebf83 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -869,14 +869,11 @@ func_printf(struct sql_context *ctx, int argc, const struct Mem *argv)
 	if (argc < 1 || mem_is_null(&argv[0]))
 		return;
 	if (argc == 1 || !mem_is_str(&argv[0])) {
-		uint32_t size = mem_snprintf(NULL, 0, &argv[0]);
-		char *str = sqlDbMallocRawNN(sql_get(), size + 1);
-		if (str == NULL) {
+		char *str = mem_strdup(&argv[0]);
+		if (str == NULL)
 			ctx->is_aborted = true;
-			return;
-		}
-		mem_snprintf(str, size + 1, &argv[0]);
-		mem_set_str_allocated(ctx->pOut, str, size);
+		else
+			mem_set_str0_allocated(ctx->pOut, str);
 		return;
 	}
 	struct PrintfArguments pargs;
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index e415b8b66..234a00d57 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -119,39 +119,64 @@ mem_is_field_compatible(const struct Mem *mem, enum field_type type)
 int
 mem_snprintf(char *buf, uint32_t size, const struct Mem *mem)
 {
+	int res = -1;
 	switch (mem->type) {
 	case MEM_TYPE_NULL:
-		return snprintf(buf, size, "NULL");
+		res = snprintf(buf, size, "NULL");
+		break;
 	case MEM_TYPE_STR:
 	case MEM_TYPE_BIN:
-		return snprintf(buf, size, "%.*s", mem->n, mem->z);
+		res = snprintf(buf, size, "%.*s", mem->n, mem->z);
+		break;
 	case MEM_TYPE_INT:
-		return snprintf(buf, size, "%lld", mem->u.i);
+		res = snprintf(buf, size, "%lld", mem->u.i);
+		break;
 	case MEM_TYPE_UINT:
-		return snprintf(buf, size, "%llu",
-				(unsigned long long)mem->u.u);
+		res = snprintf(buf, size, "%llu", (unsigned long long)mem->u.u);
+		break;
 	case MEM_TYPE_DOUBLE: {
 		char str[BUF_SIZE];
 		sql_snprintf(BUF_SIZE, str, "%!.15g", mem->u.r);
-		return snprintf(buf, size, "%s", str);
+		res = snprintf(buf, size, "%s", str);
+		break;
 	}
 	case MEM_TYPE_DEC:
-		return snprintf(buf, size, "%s", decimal_str(&mem->u.d));
+		res = snprintf(buf, size, "%s", decimal_str(&mem->u.d));
+		break;
 	case MEM_TYPE_MAP:
 	case MEM_TYPE_ARRAY:
-		return mp_snprint(buf, size, mem->z);
+		res = mp_snprint(buf, size, mem->z);
+		break;
 	case MEM_TYPE_UUID:
-		return snprintf(buf, size, "%s", tt_uuid_str(&mem->u.uuid));
+		res = snprintf(buf, size, "%s", tt_uuid_str(&mem->u.uuid));
+		break;
 	case MEM_TYPE_BOOL:
-		return snprintf(buf, size, "%s", mem->u.b ? "TRUE" : "FALSE");
+		res = snprintf(buf, size, mem->u.b ? "TRUE" : "FALSE");
+		break;
 	default:
-		return snprintf(buf, size, "unknown");
+		unreachable();
 	}
+	assert(res >= 0);
+	return res;
+}
+
+char *
+mem_strdup(const struct Mem *mem)
+{
+	int size = mem_snprintf(NULL, 0, mem);
+	assert(size >= 0);
+	char *str = sqlDbMallocRawNN(sql_get(), size + 1);
+	if (str == NULL)
+		return NULL;
+	mem_snprintf(str, size + 1, mem);
+	return str;
 }
 
 const char *
 mem_str(const struct Mem *mem)
 {
+	if (mem->type == MEM_TYPE_NULL)
+		return "NULL";
 	const char *type = mem_type_to_str(mem);
 	if (mem->type == MEM_TYPE_STR) {
 		if (mem->n <= STR_VALUE_MAX_LEN)
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 75b9dc858..1a3478f9a 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -254,6 +254,13 @@ mem_is_field_compatible(const struct Mem *mem, enum field_type type);
 int
 mem_snprintf(char *buf, uint32_t size, const struct Mem *mem);
 
+/**
+ * Returns a NULL-terminated string representation of a MEM. Memory for the
+ * result was allocated using sqlDbMallocRawNN() and should be freed.
+ */
+char *
+mem_strdup(const struct Mem *mem);
+
 /**
  * Return a string that contains description of type and value of MEM. String is
  * either allocated using static_alloc() of just a static variable. This
diff --git a/src/box/sql/printf.c b/src/box/sql/printf.c
index 08e18c15d..9caed7aa0 100644
--- a/src/box/sql/printf.c
+++ b/src/box/sql/printf.c
@@ -160,13 +160,7 @@ getTextArg(PrintfArguments * p)
 {
 	if (p->nArg <= p->nUsed)
 		return 0;
-	const struct Mem *mem = &p->apArg[p->nUsed++];
-	uint32_t size = mem_snprintf(NULL, 0, mem);
-	char *str = sqlDbMallocRawNN(sql_get(), size + 1);
-	if (str == NULL)
-		return NULL;
-	mem_snprintf(str, size + 1, mem);
-	return str;
+	return mem_strdup(&p->apArg[p->nUsed++]);
 }
 
 /*


New patch:

commit 870076945970c73ed651c37088406e7c939c6294
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Fri Oct 22 11:44:06 2021 +0300

    sql: introduce mem_snprintf()
    
    This patch introduces the mem_snprintf() function, which writes the
    string representation of a MEM to buf.

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 5abaf490d..26ccebf83 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -869,9 +869,11 @@ func_printf(struct sql_context *ctx, int argc, const struct Mem *argv)
 	if (argc < 1 || mem_is_null(&argv[0]))
 		return;
 	if (argc == 1 || !mem_is_str(&argv[0])) {
-		struct Mem *mem = ctx->pOut;
-		if (mem_copy(mem, &argv[0]) != 0 || mem_to_str(mem) != 0)
+		char *str = mem_strdup(&argv[0]);
+		if (str == NULL)
 			ctx->is_aborted = true;
+		else
+			mem_set_str0_allocated(ctx->pOut, str);
 		return;
 	}
 	struct PrintfArguments pargs;
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index f67742618..234a00d57 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -116,30 +116,76 @@ mem_is_field_compatible(const struct Mem *mem, enum field_type type)
 	return field_mp_plain_type_is_compatible(type, mp_type, true);
 }
 
-const char *
-mem_str(const struct Mem *mem)
+int
+mem_snprintf(char *buf, uint32_t size, const struct Mem *mem)
 {
-	char buf[STR_VALUE_MAX_LEN];
-	const char *type = mem_type_to_str(mem);
+	int res = -1;
 	switch (mem->type) {
 	case MEM_TYPE_NULL:
-		return "NULL";
+		res = snprintf(buf, size, "NULL");
+		break;
 	case MEM_TYPE_STR:
+	case MEM_TYPE_BIN:
+		res = snprintf(buf, size, "%.*s", mem->n, mem->z);
+		break;
+	case MEM_TYPE_INT:
+		res = snprintf(buf, size, "%lld", mem->u.i);
+		break;
+	case MEM_TYPE_UINT:
+		res = snprintf(buf, size, "%llu", (unsigned long long)mem->u.u);
+		break;
+	case MEM_TYPE_DOUBLE: {
+		char str[BUF_SIZE];
+		sql_snprintf(BUF_SIZE, str, "%!.15g", mem->u.r);
+		res = snprintf(buf, size, "%s", str);
+		break;
+	}
+	case MEM_TYPE_DEC:
+		res = snprintf(buf, size, "%s", decimal_str(&mem->u.d));
+		break;
+	case MEM_TYPE_MAP:
+	case MEM_TYPE_ARRAY:
+		res = mp_snprint(buf, size, mem->z);
+		break;
+	case MEM_TYPE_UUID:
+		res = snprintf(buf, size, "%s", tt_uuid_str(&mem->u.uuid));
+		break;
+	case MEM_TYPE_BOOL:
+		res = snprintf(buf, size, mem->u.b ? "TRUE" : "FALSE");
+		break;
+	default:
+		unreachable();
+	}
+	assert(res >= 0);
+	return res;
+}
+
+char *
+mem_strdup(const struct Mem *mem)
+{
+	int size = mem_snprintf(NULL, 0, mem);
+	assert(size >= 0);
+	char *str = sqlDbMallocRawNN(sql_get(), size + 1);
+	if (str == NULL)
+		return NULL;
+	mem_snprintf(str, size + 1, mem);
+	return str;
+}
+
+const char *
+mem_str(const struct Mem *mem)
+{
+	if (mem->type == MEM_TYPE_NULL)
+		return "NULL";
+	const char *type = mem_type_to_str(mem);
+	if (mem->type == MEM_TYPE_STR) {
 		if (mem->n <= STR_VALUE_MAX_LEN)
 			return tt_sprintf("%s('%.*s')", type, mem->n, mem->z);
 		return tt_sprintf("%s('%.*s...)", type, STR_VALUE_MAX_LEN,
 				  mem->z);
-	case MEM_TYPE_INT:
-		return tt_sprintf("%s(%lld)", type, mem->u.i);
-	case MEM_TYPE_UINT:
-		return tt_sprintf("%s(%llu)", type, mem->u.u);
-	case MEM_TYPE_DOUBLE:
-		sql_snprintf(STR_VALUE_MAX_LEN, buf, "%!.15g", mem->u.r);
-		return tt_sprintf("%s(%s)", type, buf);
-	case MEM_TYPE_DEC:
-		decimal_to_string(&mem->u.d, buf);
-		return tt_sprintf("%s(%s)", type, buf);
-	case MEM_TYPE_BIN: {
+	}
+	char buf[STR_VALUE_MAX_LEN];
+	if (mem->type == MEM_TYPE_BIN) {
 		int len = MIN(mem->n, STR_VALUE_MAX_LEN / 2);
 		for (int i = 0; i < len; ++i) {
 			int n = (mem->z[i] & 0xF0) >> 4;
@@ -151,24 +197,10 @@ mem_str(const struct Mem *mem)
 			return tt_sprintf("%s(x'%.*s...)", type, len * 2, buf);
 		return tt_sprintf("%s(x'%.*s')", type, len * 2, buf);
 	}
-	case MEM_TYPE_MAP:
-	case MEM_TYPE_ARRAY: {
-		const char *str = mp_str(mem->z);
-		uint32_t len = strlen(str);
-		uint32_t minlen = MIN(STR_VALUE_MAX_LEN, len);
-		memcpy(buf, str, minlen);
-		if (len <= STR_VALUE_MAX_LEN)
-			return tt_sprintf("%s(%.*s)", type, minlen, buf);
-		return tt_sprintf("%s(%.*s...)", type, minlen, buf);
-	}
-	case MEM_TYPE_UUID:
-		tt_uuid_to_string(&mem->u.uuid, buf);
+	int size = mem_snprintf(buf, STR_VALUE_MAX_LEN, mem);
+	if (size <= STR_VALUE_MAX_LEN)
 		return tt_sprintf("%s(%s)", type, buf);
-	case MEM_TYPE_BOOL:
-		return tt_sprintf("%s(%s)", type, mem->u.b ? "TRUE" : "FALSE");
-	default:
-		return "unknown";
-	}
+	return tt_sprintf("%s(%.*s...)", type, STR_VALUE_MAX_LEN, buf);
 }
 
 static const char *
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 9d5245708..1a3478f9a 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -246,6 +246,21 @@ mem_is_any_null(const struct Mem *mem1, const struct Mem *mem2)
 bool
 mem_is_field_compatible(const struct Mem *mem, enum field_type type);
 
+/**
+ * Write a NULL-terminated string representation of a MEM to buf. Returns the
+ * number of bytes required to write the value, excluding '\0'. If the return
+ * value is equal to or greater than size, then the value has been truncated.
+ */
+int
+mem_snprintf(char *buf, uint32_t size, const struct Mem *mem);
+
+/**
+ * Returns a NULL-terminated string representation of a MEM. Memory for the
+ * result was allocated using sqlDbMallocRawNN() and should be freed.
+ */
+char *
+mem_strdup(const struct Mem *mem);
+
 /**
  * Return a string that contains description of type and value of MEM. String is
  * either allocated using static_alloc() of just a static variable. This
diff --git a/src/box/sql/printf.c b/src/box/sql/printf.c
index 8da7c9878..9caed7aa0 100644
--- a/src/box/sql/printf.c
+++ b/src/box/sql/printf.c
@@ -160,20 +160,7 @@ getTextArg(PrintfArguments * p)
 {
 	if (p->nArg <= p->nUsed)
 		return 0;
-	struct Mem mem;
-	mem_create(&mem);
-	mem_copy_as_ephemeral(&mem, &p->apArg[p->nUsed++]);
-	if (mem_to_str(&mem) != 0) {
-		mem_destroy(&mem);
-		return NULL;
-	}
-	char *str = sqlDbMallocRawNN(sql_get(), mem.n + 1);
-	if (str == NULL)
-		return NULL;
-	memcpy(str, mem.z, mem.n);
-	str[mem.n] = '\0';
-	mem_destroy(&mem);
-	return str;
+	return mem_strdup(&p->apArg[p->nUsed++]);
 }
 
 /*
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index a9aa5acf7..6cecd7c6d 100755
--- a/test/sql-tap/sql-errors.test.lua
+++ b/test/sql-tap/sql-errors.test.lua
@@ -812,7 +812,8 @@ test:do_catchsql_test(
 	"SELECT CAST(a AS UNSIGNED) from test;", {
 		1, 'Type mismatch: can not convert array(["aaaaaaaaaaaaaaaaaa'..
 		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'..
-		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...) to unsigned'
+		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...) to '..
+		'unsigned'
 	})
 
 test:do_catchsql_test(
@@ -820,7 +821,7 @@ test:do_catchsql_test(
 	"SELECT CAST(m AS UNSIGNED) from test;", {
 		1, 'Type mismatch: can not convert map({"a": 1, "b": "aaaaaaa'..
 		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'..
-		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...) to unsigned'
+		'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...) to unsigned'
 	})
 
 test:execsql('DROP TABLE test;')
diff --git a/test/sql/types.result b/test/sql/types.result
index 947795b03..5a822fddf 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -1623,7 +1623,7 @@ box.execute('INSERT INTO t1(a) SELECT a FROM t2;')
 - null
 - 'Type mismatch: can not convert array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13,
   14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33,
-  34, ...) to scalar'
+  34,...) to scalar'
 ...
 s:drop()
 ---

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

* Re: [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf()
  2021-11-15 16:06 [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf() Mergen Imeev via Tarantool-patches
  2021-11-15 16:06 ` [Tarantool-patches] [PATCH v1 1/2] sql: omit quotes for UUID values in errors Mergen Imeev via Tarantool-patches
  2021-11-15 16:06 ` [Tarantool-patches] [PATCH v1 2/2] sql: introduce mem_snprintf() Mergen Imeev via Tarantool-patches
@ 2021-11-21 15:27 ` Vladislav Shpilevoy via Tarantool-patches
  2 siblings, 0 replies; 8+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-11-21 15:27 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

Thanks for the fixes!

LGTM.

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

* Re: [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf()
  2021-11-22  8:22 Mergen Imeev via Tarantool-patches
@ 2021-11-24  8:54 ` Kirill Yukhin via Tarantool-patches
  0 siblings, 0 replies; 8+ messages in thread
From: Kirill Yukhin via Tarantool-patches @ 2021-11-24  8:54 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

Hello,

On 22 ноя 11:22, imeevma@tarantool.org wrote:
> This patch-set introduces function mem_snprintf() which can be used to get
> string representation of a MEM.
> 
> https://github.com/tarantool/tarantool/issues/4145
> https://github.com/tarantool/tarantool/tree/imeevma/gh-4145-follow-up

LGTM.
I've checked your patchset into master.

--
Regards, Kirill Yukhin

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

* [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf()
@ 2021-11-22  8:22 Mergen Imeev via Tarantool-patches
  2021-11-24  8:54 ` Kirill Yukhin via Tarantool-patches
  0 siblings, 1 reply; 8+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-11-22  8:22 UTC (permalink / raw)
  To: kyukhin; +Cc: tarantool-patches

This patch-set introduces function mem_snprintf() which can be used to get
string representation of a MEM.

https://github.com/tarantool/tarantool/issues/4145
https://github.com/tarantool/tarantool/tree/imeevma/gh-4145-follow-up

Mergen Imeev (2):
  sql: omit quotes for UUID values in errors
  sql: introduce mem_snprintf()

 src/box/sql/func.c                            |   6 +-
 src/box/sql/mem.c                             | 100 ++++++++++++------
 src/box/sql/mem.h                             |  15 +++
 src/box/sql/printf.c                          |  15 +--
 test/sql-tap/cast.test.lua                    |   4 +-
 test/sql-tap/decimal.test.lua                 |   6 +-
 .../gh-5364-define-bit-wise-rules.test.lua    |   8 +-
 ...-5756-implicit-cast-in-arithmetic.test.lua |  10 +-
 test/sql-tap/sql-errors.test.lua              |   7 +-
 test/sql-tap/uuid.test.lua                    |  58 +++++-----
 test/sql/types.result                         |   2 +-
 11 files changed, 134 insertions(+), 97 deletions(-)

-- 
2.25.1


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

end of thread, other threads:[~2021-11-24  8:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-15 16:06 [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf() Mergen Imeev via Tarantool-patches
2021-11-15 16:06 ` [Tarantool-patches] [PATCH v1 1/2] sql: omit quotes for UUID values in errors Mergen Imeev via Tarantool-patches
2021-11-15 16:06 ` [Tarantool-patches] [PATCH v1 2/2] sql: introduce mem_snprintf() Mergen Imeev via Tarantool-patches
2021-11-17 23:03   ` Vladislav Shpilevoy via Tarantool-patches
2021-11-19 12:02     ` Mergen Imeev via Tarantool-patches
2021-11-21 15:27 ` [Tarantool-patches] [PATCH v1 0/2] Introduce mem_snprintf() Vladislav Shpilevoy via Tarantool-patches
2021-11-22  8:22 Mergen Imeev via Tarantool-patches
2021-11-24  8:54 ` Kirill Yukhin 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