From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp62.i.mail.ru (smtp62.i.mail.ru [217.69.128.42])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 2682D445320
 for <tarantool-patches@dev.tarantool.org>;
 Sun,  5 Jul 2020 20:03:49 +0300 (MSK)
References: <1593757997-4145-1-git-send-email-alyapunov@tarantool.org>
 <1593757997-4145-3-git-send-email-alyapunov@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <0ce23cdd-5a37-e1a3-3b3c-c949120c9e96@tarantool.org>
Date: Sun, 5 Jul 2020 19:03:47 +0200
MIME-Version: 1.0
In-Reply-To: <1593757997-4145-3-git-send-email-alyapunov@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH 02/15] Check data_offset overflow in
 struct tuple
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Aleksandr Lyapunov <alyapunov@tarantool.org>, tarantool-patches@dev.tarantool.org

Thanks for the patch!

See 3 comments below.

> diff --git a/test/box/huge_field_map.result b/test/box/huge_field_map.result
> new file mode 100644
> index 0000000..11b4da3
> --- /dev/null
> +++ b/test/box/huge_field_map.result
> @@ -0,0 +1,49 @@
> +-- test-run result file version 2
> +env = require('test_run')
> + | ---
> + | ...
> +test_run = env.new()
> + | ---
> + | ...
> +
> +s = box.schema.space.create('test', {engine = 'memtx'})
> + | ---
> + | ...
> +i1 = s:create_index('pk')
> + | ---
> + | ...
> +i2 = s:create_index('mk', {parts={{'[2][*]', 'uint'}}})

1. Why do you need a multikey index? This test lacks a comment.

> + | ---
> + | ...
> +test_run:cmd("setopt delimiter ';'")
> + | ---
> + | - true
> + | ...
> diff --git a/test/box/huge_field_map_long.result b/test/box/huge_field_map_long.result
> new file mode 100644
> index 0000000..d7971ae
> --- /dev/null
> +++ b/test/box/huge_field_map_long.result
> @@ -0,0 +1,51 @@
> +-- test-run result file version 2
> +env = require('test_run')
> + | ---
> + | ...
> +test_run = env.new()
> + | ---
> + | ...
> +
> +s = box.schema.space.create('test', {engine = 'memtx'})

2. I saw you also fixed vinyl and runtime tuples. I suppose
you need to test them as well.

> + | ---
> + | ...
> +test_run:cmd("setopt delimiter ';'")
> + | ---
> + | - true
> + | ...
> +function test()
> +    local t = {}
> +    local k = {}
> +    for i = 1,128 do
> +        local parts = {}
> +        for j = 0,127 do
> +            table.insert(parts, {i * 128 - j, 'uint'})
> +            table.insert(t, 1)
> +        end
> +        if i == 1 then k = table.deepcopy(t) end
> +        s:create_index('test'..i, {parts = parts})
> +        if i % 16 == 0 then
> +            s:replace(t)
> +            s:delete(k)
> +        end
> +    end
> +end;
> + | ---
> + | ...
> +test_run:cmd("setopt delimiter ''");
> + | ---
> + | - true
> + | ...
> +
> +pcall(test) -- must fail but not crash
> + | ---
> + | - false
> + | - 'Can''t create tuple: metadata size 65542 is too big'
> + | ...
> +
> +test = nil
> + | ---
> + | ...
> +s:drop()
> + | ---
> + | ...
> diff --git a/test/box/huge_field_map_long.test.lua b/test/box/huge_field_map_long.test.lua
> new file mode 100644

3. Why do you need a 'long' test? The first test seems to be good enough.