On Sat, Mar 25, 2017 at 12:33 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> On Sat, Mar 25, 2017 at 11:02 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Thu, Mar 23, 2017 at 11:24 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>>> Hi,
>>>
>>> On Tue, Feb 7, 2017 at 9:23 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>>>> On Mon, Feb 6, 2017 at 10:40 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>>>>> Maybe we should call them "unused pages".
>>>>>
>>>>> +1. If we consider some more names for that column then probably one
>>>>> alternative could be "empty pages".
>>>>
>>>> Yeah, but I think "unused" might be better. Because a page could be
>>>> in use (as an overflow page or primary bucket page) and still be
>>>> empty.
>>>>
>>>
>>> Based on the earlier discussions, I have prepared a patch that would
>>> allow pgstathashindex() to show the number of unused pages in hash
>>> index. Please find the attached patch for the same. Thanks.
>>>
>>
>> else if (opaque->hasho_flag & LH_BITMAP_PAGE)
>> stats.bitmap_pages++;
>> + else if (PageIsEmpty(page))
>> + stats.unused_pages++;
>>
>> I think having this check after PageIsNew() makes more sense then
>> having at the place where you currently have,
>
> I don't think having it just below PageIsNew() check would be good.
> The reason being, there can be a bucket page which is in use but still
> be empty. So, if we add a check just below PageIsNew check then even
> though a page is marked as bucket page and is empty it will be shown
> as unused page which i feel is not correct. Here is one simple example
> that illustrates this point,
>
oh, is it a page where all the items have been deleted and no new
items have been inserted? The reason why I have told that place is
not appropriate is because all the other checks in near by code are on
the page type and this check looks odd at that place, but we might
need to do this way if there is no other clean solution.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
