On 2021-08-18 1:29 a.m., Kyotaro Horiguchi wrote:
> At Tue, 17 Aug 2021 17:04:44 +0900, Michael Paquier <michael@paquier.xyz> wrote in
>> On Fri, Jul 02, 2021 at 06:22:56PM -0300, Ranier Vilela wrote:
>>> Em qui., 1 de jul. de 2021 às 17:20, Mahendra Singh Thalor <
>>> mahi6run@gmail.com> escreveu:
>>>> Please can we try to hit this rare condition by any test case. If you have
>>>> any test cases, please share.
>> Yeah, this needs to be proved. Are you sure that this change is
>> actually right? The bottom of FreePageManagerPutInternal() has
>> assumptions that a page may not be found during a btree search, with
>> an index value used.
> By a quick look, FreePageBtreeSearch is called only from
> FreePageManagerPutInternal at three points. The first one assumes that
> result.found == true, at the rest points are passed only when
> fpm->btree_depth > 0, i.e, fpm->btree_root is non-NULL.
>
> In short FreePageBtreeSeach is never called when fpm->btree_root is
> NULL. I don't think we need to fill-in other members since the
> contract of the function looks fine.
>
> It might be simpler to turn 'if (btp == NULL)' to an assertion.
After added the initialization of split_pages in patch
fix_unitialized_var_index_freepage-v1.patch,
+ result->split_pages = 0;
it actually changed the assertion condition after the second time
function call of FreePageBtreeSearch.
FreePageBtreeSearch(fpm, first_page, &result);
/*
* The act of allocating pages for use in constructing our
btree
* should never cause any page to become more full, so the new
* split depth should be no greater than the old one, and
perhaps
* less if we fortuitously allocated a chunk that freed up
a slot
* on the page we need to update.
*/
Assert(result.split_pages <= fpm->btree_recycle_count);
Should we consider adding some test cases to make sure this assertion
will still function properly?
>
> regards.
>
--
David
Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca
