[tarantool-patches] Re: [PATCH v1 2/2] sql: enable multi-engine tests for SQL

[Vladislav Shpilevoy]

Thanks for the fixes! See 2 comments below.

On 26/06/2018 15:22, Kirill Shcherbatov wrote:
>> Did you test sql-tap/ suite? Why is not it in the
>> patch? It could be updated automatically using a
>> short script that adds the engine setting at the
>> beginning of a file.
> Ok, I've change constructor to start SQL-TAP test with all engines.
> This patch should be merged after than "Vinyl read iterator skips tuples #3477"
> 
>> 1. Just increase vinyl_max_tuple_size to the same value. This
>> test should work both for vinyl and memtx.
> Ok.
> +-- 4 * 1024 * 1024 is higher than vinyl max tuple size
> +old_tuple_size = box.cfg.vinyl_max_tuple_size
> +box.cfg{vinyl_max_tuple_size = 5 * 1024 * 1024}

1. Not in the test. See where memtx_max_tuple_size is changed.

> +_ = space:replace{1, 1, string.rep('a', 4 * 1024 * 1024) }

2. This line it unnecessary diff.

> +box.cfg{vinyl_max_tuple_size = old_tuple_size}
> 
>> 2. Vinyl rollbacks failed transactions, but it does not
>> rollback the whole transaction on a non-caught exception.
> Ok.
> +box.rollback()
>