* [Tarantool-patches] [PATCH v1 1/1] sql: fix quote() function
@ 2021-08-17 13:37 Mergen Imeev via Tarantool-patches
2021-08-18 17:18 ` Igor Munkin via Tarantool-patches
0 siblings, 1 reply; 4+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-08-17 13:37 UTC (permalink / raw)
To: imun; +Cc: tarantool-patches
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
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
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 1622104d3..2aa637fbc 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1096,26 +1096,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 94b0bb1f6..3196c4054 100644
--- a/test/sql-tap/engine.cfg
+++ b/test/sql-tap/engine.cfg
@@ -23,6 +23,9 @@
"gh-6164-uuid-follow-ups.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..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
+ })
+
+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 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]
+ - [18000000000000000000]
...
box.execute("SELECT LEAST(d, 0) FROM t;")
---
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: fix quote() function
2021-08-17 13:37 [Tarantool-patches] [PATCH v1 1/1] sql: fix quote() function Mergen Imeev via Tarantool-patches
@ 2021-08-18 17:18 ` Igor Munkin via Tarantool-patches
2021-08-18 19:04 ` Mergen Imeev via Tarantool-patches
0 siblings, 1 reply; 4+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2021-08-18 17:18 UTC (permalink / raw)
To: imeevma; +Cc: tarantool-patches
Mergen,
Thanks for the patch! Please consider minor comments below.
On 17.08.21, imeevma@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?
> 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?
> + })
> +
> +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?
> + - [18000000000000000000]
> ...
> box.execute("SELECT LEAST(d, 0) FROM t;")
> ---
> --
> 2.25.1
>
--
Best regards,
IM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: fix quote() function
2021-08-18 19:04 ` Mergen Imeev via Tarantool-patches
@ 2021-08-18 19:03 ` Igor Munkin via Tarantool-patches
0 siblings, 0 replies; 4+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2021-08-18 19:03 UTC (permalink / raw)
To: Mergen Imeev; +Cc: tarantool-patches
Mergen,
Thanks for the fixes! The new patch below LGTM. I also tested the fix on
my M1 machine, and the issue is gone. JFYI, the whole sql-tap suite
works fine after your patch (with all LuaJIT-related patched applied).
On 18.08.21, Mergen Imeev wrote:
> 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:
<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.
Great, thanks!
>
> > > + })
> > > +
> > > +test:finish_test()
<snipped>
> > > 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.
Got it, thanks!
>
> > > + - [18000000000000000000]
> > > ...
> > > box.execute("SELECT LEAST(d, 0) FROM t;")
> > > ---
> > > --
> > > 2.25.1
> > >
> >
> > --
> > Best regards,
> > IM
>
>
> New patch:
>
>
> commit 2e2d979c53be8d9f97071d3003e1069fc8c078cf
> Author: Mergen Imeev <imeevma@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
Typo: s/Ater/After/.
> case DOUBLE values is given. If the argument is not number, string
Typo: s/values/value/.
> 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.
>
<snipped>
> ---
--
Best regards,
IM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] sql: fix quote() function
2021-08-18 17:18 ` Igor Munkin via Tarantool-patches
@ 2021-08-18 19:04 ` Mergen Imeev via Tarantool-patches
2021-08-18 19:03 ` Igor Munkin via Tarantool-patches
0 siblings, 1 reply; 4+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-08-18 19:04 UTC (permalink / raw)
To: Igor Munkin; +Cc: tarantool-patches
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@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@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;")
---
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-18 19:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-17 13:37 [Tarantool-patches] [PATCH v1 1/1] sql: fix quote() function Mergen Imeev via Tarantool-patches
2021-08-18 17:18 ` Igor Munkin via Tarantool-patches
2021-08-18 19:04 ` Mergen Imeev via Tarantool-patches
2021-08-18 19:03 ` Igor Munkin 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