Re: Issue in _bt_getrootheight

From: Gurjeet Singh <gurjeet(at)singh(dot)im>
To: Ahmed Ibrahim <ahmed(dot)ibr(dot)hashim(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Issue in _bt_getrootheight
Date: 2023-07-20 03:45:41
Message-ID: CABwTF4XSREc4f-OxMXWnO=d3RPud+ntibPNS0rhvYinxMSv0sg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 11, 2023 at 9:35 AM Ahmed Ibrahim
<ahmed(dot)ibr(dot)hashim(at)gmail(dot)com> wrote:
>
> We have been working on the pg_adviser extension whose goal is to suggest indexes by creating virtual/hypothetical indexes and see how it affects the query cost.
>
> The hypothetical index shouldn't take any space on the disk (allocates 0 pages) so we give it the flag INDEX_CREATE_SKIP_BUILD.
> But the problem comes from here when the function get_relation_info is called in planning stage, it tries to calculate the B-Tree height by calling function _bt_getrootheight, but the B-Tree is not built at all, and its metadata page (which is block 0 in our case) doesn't exist, so this returns error that it cannot read the page (since it doesn't exist).
>
> I tried to debug the code and found that this feature was introduced in version 9.3 under this commit [1]. I think that in the code we need to check if it's a B-Tree index AND the index is built/have some pages, then we can go and calculate it otherwise just put it to -1

> I mean instead of this
> if (info->relam == BTREE_AM_OID)
> {
> /* For btrees, get tree height while we have the index open */
> info->tree_height = _bt_getrootheight(indexRelation);
> }
> else
> {
> /* For other index types, just set it to "unknown" for now */
> info->tree_height = -1;
> }
>
> The first line should be
> if (info->relam == BTREE_AM_OID && info->pages > 0)
> or use the storage manager (smgr) to know if the first block exists.

I think the better method would be to calculate the index height
*after* get_relation_info_hook is called. That way, instead of the
server guessing whether or not an index is hypothetical it can rely on
the index adviser's notion of which index is hypothetical. The hook
implementer has the opportunity to not only mark the
indexOptInfo->hypothetical = true, but also calculate the tree height,
if they can.

Please see attached the patch that does this. Let me know if this patch helps.

Best regards,
Gurjeet
http://Gurje.et

Attachment Content-Type Size
calculate-index-height-after-calling-get_relation_info_hook.patch application/octet-stream 3.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-07-20 03:55:43 Re: Support worker_spi to execute the function dynamically.
Previous Message Amit Kapila 2023-07-20 03:41:40 Re: logicalrep_message_type throws an error