Re: [Tarantool-patches] [PATCH v1 1/1] sql: fix quote() function

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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