[Tarantool-patches] [PATCH v1 1/1] sql: fix quote() function
Mergen Imeev
imeevma at tarantool.org
Wed Aug 18 22:04:32 MSK 2021
Hi! Thank you for the review! My answers and new patch below. I did not include
diff, since most of changes were in commit-message and there were only couple
changes lines in the test.
On Wed, Aug 18, 2021 at 08:18:40PM +0300, Igor Munkin wrote:
> Mergen,
>
> Thanks for the patch! Please consider minor comments below.
>
> On 17.08.21, imeevma at tarantool.org wrote:
> > Ater this patch SQL built-in function QUOTE() will return the same
> > DOUBLE in case it receives DOUBLE as an argument. If the argument is
>
> Minor: it's better to say QUOTE() returns the argument in case DOUBLE
> value is given. Since we're here: do we need a docbot request for this?
>
Fixed. Added a docbot request.
> > not number, string representation of the argument will be returned.
> >
> > Closes #6239
> > ---
> > https://github.com/tarantool/tarantool/issues/6239
> > https://github.com/tarantool/tarantool/tree/imeevma/gh-6239-quote-with-double-arg
> >
> > src/box/sql/func.c | 15 +--------------
> > test/sql-tap/engine.cfg | 3 +++
> > .../gh-6239-quote-with-double-arg.test.lua | 14 ++++++++++++++
> > test/sql-tap/trigger5.test.lua | 3 ++-
> > test/sql/types.result | 8 ++++----
> > 5 files changed, 24 insertions(+), 19 deletions(-)
> > create mode 100755 test/sql-tap/gh-6239-quote-with-double-arg.test.lua
> >
>
> <snipped>
>
> > diff --git a/test/sql-tap/gh-6239-quote-with-double-arg.test.lua b/test/sql-tap/gh-6239-quote-with-double-arg.test.lua
> > new file mode 100755
> > index 000000000..60f85f20f
> > --- /dev/null
> > +++ b/test/sql-tap/gh-6239-quote-with-double-arg.test.lua
> > @@ -0,0 +1,14 @@
> > +#!/usr/bin/env tarantool
> > +local test = require("sqltester")
> > +test:plan(1)
> > +
> > +-- Make sure that QUOTE() returns DOUBLE in cast it receives DOUBLE.
> > +test:do_execsql_test(
> > + "gh-6239",
> > + [[
> > + SELECT QUOTE(1.5);
> > + ]], {
> > + 1.5
>
> Do we need to check QUOTE preserves the type of the argument?
>
Added check with typeof. I see no sense to add check with metadata, since there
we will see "string". It will be fixed in patch-set about SQL built-in
functions.
> > + })
> > +
> > +test:finish_test()
> > diff --git a/test/sql-tap/trigger5.test.lua b/test/sql-tap/trigger5.test.lua
> > index 8336cdcf2..7f3762126 100755
> > --- a/test/sql-tap/trigger5.test.lua
> > +++ b/test/sql-tap/trigger5.test.lua
> > @@ -31,7 +31,8 @@ test:do_execsql_test(
> > INSERT INTO Undo VALUES
> > ((SELECT coalesce(max(id),0) + 1 FROM Undo),
> > (SELECT 'INSERT INTO Item (a,b,c) VALUES (' || CAST(coalesce(old.a,'NULL') AS TEXT)
> > - || ',' || quote(old.b) || ',' || CAST(old.c AS TEXT) || ');'));
> > + || ',' || CAST(quote(old.b) AS STRING) || ',' ||
> > + CAST(old.c AS TEXT) || ');'));
>
> [Hm. Expecting nothing is completely broken, since test results are not
> changed.]
>
> > END;
> > DELETE FROM Item WHERE a = 1;
> > SELECT * FROM Undo;
> > diff --git a/test/sql/types.result b/test/sql/types.result
> > index 07d5b46e4..358667e94 100644
> > --- a/test/sql/types.result
> > +++ b/test/sql/types.result
> > @@ -1908,10 +1908,10 @@ box.execute("SELECT quote(d) FROM t;")
> > - name: COLUMN_1
> > type: string
> > rows:
> > - - ['10.0']
> > - - ['-2.0']
> > - - ['3.3']
> > - - ['1.8e+19']
> > + - [10]
> > + - [-2]
> > + - [3.3]
>
> Fraction is dropped by YAML serializer, right?
>
Yes.
> > + - [18000000000000000000]
> > ...
> > box.execute("SELECT LEAST(d, 0) FROM t;")
> > ---
> > --
> > 2.25.1
> >
>
> --
> Best regards,
> IM
New patch:
commit 2e2d979c53be8d9f97071d3003e1069fc8c078cf
Author: Mergen Imeev <imeevma at gmail.com>
Date: Fri Aug 13 05:50:06 2021 +0300
sql: fix quote() function
Ater this patch SQL built-in function QUOTE() will return argument in
case DOUBLE values is given. If the argument is not number, string
representation of the argument will be returned.
Closes #6239
@TarantoolBot document
Title: QUOTE() and DOUBLE argument
After this patch function QUOTE() will return argument in case it
receives DOUBLE value as an argument. The same for all other numeric
types. In case it was given value of non-numeric type, it will return
string representation of the argument.
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 1551d3ef2..3da608c0b 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1091,26 +1091,13 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
assert(argc == 1);
UNUSED_PARAMETER(argc);
switch (argv[0]->type) {
- case MEM_TYPE_DOUBLE:{
- double r1, r2;
- char zBuf[50];
- r1 = mem_get_double_unsafe(argv[0]);
- sql_snprintf(sizeof(zBuf), zBuf, "%!.15g", r1);
- sqlAtoF(zBuf, &r2, 20);
- if (r1 != r2) {
- sql_snprintf(sizeof(zBuf), zBuf, "%!.20e",
- r1);
- }
- sql_result_text(context, zBuf, -1,
- SQL_TRANSIENT);
- break;
- }
case MEM_TYPE_UUID: {
char buf[UUID_STR_LEN + 1];
tt_uuid_to_string(&argv[0]->u.uuid, &buf[0]);
sql_result_text(context, buf, UUID_STR_LEN, SQL_TRANSIENT);
break;
}
+ case MEM_TYPE_DOUBLE:
case MEM_TYPE_UINT:
case MEM_TYPE_INT: {
sql_result_value(context, argv[0]);
diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
index 820c72b00..5799780dc 100644
--- a/test/sql-tap/engine.cfg
+++ b/test/sql-tap/engine.cfg
@@ -26,6 +26,9 @@
"metatypes.test.lua": {
"memtx": {"engine": "memtx"}
},
+ "gh-6239-quote-with-double-arg.test.lua": {
+ "memtx": {"engine": "memtx"}
+ },
"gh-4077-iproto-execute-no-bind.test.lua": {},
"*": {
"memtx": {"engine": "memtx"},
diff --git a/test/sql-tap/gh-6239-quote-with-double-arg.test.lua b/test/sql-tap/gh-6239-quote-with-double-arg.test.lua
new file mode 100755
index 000000000..dab12253e
--- /dev/null
+++ b/test/sql-tap/gh-6239-quote-with-double-arg.test.lua
@@ -0,0 +1,14 @@
+#!/usr/bin/env tarantool
+local test = require("sqltester")
+test:plan(1)
+
+-- Make sure that QUOTE() returns DOUBLE in cast it receives DOUBLE.
+test:do_execsql_test(
+ "gh-6239",
+ [[
+ SELECT QUOTE(1.5), TYPEOF(QUOTE(1.5));
+ ]], {
+ 1.5, "double"
+ })
+
+test:finish_test()
diff --git a/test/sql-tap/trigger5.test.lua b/test/sql-tap/trigger5.test.lua
index 8336cdcf2..7f3762126 100755
--- a/test/sql-tap/trigger5.test.lua
+++ b/test/sql-tap/trigger5.test.lua
@@ -31,7 +31,8 @@ test:do_execsql_test(
INSERT INTO Undo VALUES
((SELECT coalesce(max(id),0) + 1 FROM Undo),
(SELECT 'INSERT INTO Item (a,b,c) VALUES (' || CAST(coalesce(old.a,'NULL') AS TEXT)
- || ',' || quote(old.b) || ',' || CAST(old.c AS TEXT) || ');'));
+ || ',' || CAST(quote(old.b) AS STRING) || ',' ||
+ CAST(old.c AS TEXT) || ');'));
END;
DELETE FROM Item WHERE a = 1;
SELECT * FROM Undo;
diff --git a/test/sql/types.result b/test/sql/types.result
index 68bdcd62e..fa78e8d9a 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -1909,10 +1909,10 @@ box.execute("SELECT quote(d) FROM t;")
- name: COLUMN_1
type: string
rows:
- - ['10.0']
- - ['-2.0']
- - ['3.3']
- - ['1.8e+19']
+ - [10]
+ - [-2]
+ - [3.3]
+ - [18000000000000000000]
...
box.execute("SELECT LEAST(d, 0) FROM t;")
---
More information about the Tarantool-patches
mailing list