From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <kshcherbatov@tarantool.org>
Subject: Re: [tarantool-patches] Re: [PATCH v5 3/4] salad: introduce
 bps_tree_delete_identical routine
References: <cover.1557142159.git.kshcherbatov@tarantool.org>
 <9a18d47fe9c8f9a8e0bc98de9aec98f8c7723216.1557142159.git.kshcherbatov@tarantool.org>
 <20190506143445.redgeum66gy7onex@esperanza>
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Message-ID: <d083b455-b2dc-e3b3-ef9e-380dc48d945f@tarantool.org>
Date: Mon, 6 May 2019 17:55:36 +0300
MIME-Version: 1.0
In-Reply-To: <20190506143445.redgeum66gy7onex@esperanza>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
To: tarantool-patches@freelists.org, Vladimir Davydov <vdavydov.dev@gmail.com>
List-ID: <tarantool-patches.dev.tarantool.org>


>>  #ifndef BPS_TREE_IDENTICAL
>>  #define BPS_TREE_IDENTICAL(a, b) (a == b)
>> +#define BPS_TREE_IDENTICAL_STD 1
> 
> This looks ugly. Let's require the caller to always define
> BPS_TREE_IDENTICAL - it's not a big deal.

I have considered this option, maybe it is better.
Done.

>> +/**
>> + * @brief Delete an identical element from a tree (unlike
>> + * bps_tree_delete this routine relies on BPS_TREE_IDENTICAL
>> + * instead of BPS_TREE_COMPARE).
>> + * @param tree - pointer to a tree
>> + * @param elem - the element tot delete
>> + * @return - true on success or false if the element was not found in tree
> 
> I guess it should also return -1 if the element was found but not
> deleted, in other words it should return 0 iff the element has been
> deleted.

	Done:

	struct bps_leaf *leaf = leaf_path_elem.block;
	if (!BPS_TREE_IDENTICAL(elem,
				leaf->elems[leaf_path_elem.insertion_point]))
		return -1;
	bps_tree_process_delete_leaf(tree, &leaf_path_elem);

> 
> Could you please reuse the existing tree for the test rather than
> introducing a new one?

Unfortunately, it is not possible. All other tests don't use structure as a leaf.
> Please check the return value as well.

	if (struct_tree_delete_identical(&tree, e2) == 0)
		fail("deletion of the non-identical element must fail", "false");
	if (struct_tree_find(&tree, 1) == NULL)
		fail("test non-identical element deletion failure", "false");
	if (struct_tree_delete_identical(&tree, e1) != 0)
		fail("deletion of the identical element must not fail", "false");
	if (struct_tree_find(&tree, 1) != NULL)
		fail("test identical element deletion completion", "false");

============================================

Updated on branch.