[tarantool-patches] Re: [PATCH v1 1/1] sql: decrease SELECT_COMPOUND_LIMIT threshold

n.pettik korablev at tarantool.org
Thu Sep 20 11:49:01 MSK 2018


>> Hard limit 500 seems to be too much. Lets set hard limit 50 (as it was before).
>> If user want to set more than 50, he can set it to 0 (according to comments
>> it means no limit at all).
> Ok, set 50.
> 
>> Ok, you may skip it, but anyway, I would add tests where you set
>> limit to 0 and process compound select with 31 parts.
> Introduced such test.
> 
>> You forgot to fix commit message. See previous review comments.
> Updated.
> 
>> This pragma status is not displayed via simple ‘pragma’.
> It is ok, the other non-boolean pragmas have same behavior. 
> Displaying sql_default_engine was a big mistake. I've just implemented requirement
> from review comment that time, but it was not correct. It looks out-of-order,
> as an exception in code there.

I don’t think so. It looks OK and it (default engine) seems to
be very useful for user. However, I don’t insist on this change,
so let it be as is.

> @@ -193,7 +193,7 @@ test:do_execsql_test(
>         INSERT INTO t3 VALUES(56.0);
>     ]], {
>         -- <select7-7.1>
> -        
> +
>         -- </select7-7.1>
>     })
> 
> @@ -216,7 +216,7 @@ test:do_execsql_test(
>         INSERT INTO t4 VALUES( 3.0 );
>     ]], {
>         -- <select7-7.3>
> -        
> +
>         -- </select7-7.3>
>     })
> 
> @@ -233,7 +233,7 @@ test:do_execsql_test(
> test:do_execsql_test(
>     "select7-7.5",
>     [[
> -        SELECT a=0, typeof(a) FROM t4 
> +        SELECT a=0, typeof(a) FROM t4
>     ]], {
>         -- <select7-7.5>
>         0, "real", 0, "real"
> @@ -243,7 +243,7 @@ test:do_execsql_test(
> test:do_execsql_test(
>     "select7-7.6",
>     [[
> -        SELECT a=0, typeof(a) FROM t4 GROUP BY a 
> +        SELECT a=0, typeof(a) FROM t4 GROUP BY a
>     ]], {

Garbage diff.

Otherwise, patch LGTM.





More information about the Tarantool-patches mailing list