From: | Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> |
---|---|
To: | Aleksander Alekseev <aleksander(at)tigerdata(dot)com> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org, Maxime Schoemans <maxime(dot)schoemans(at)enterprisedb(dot)com> |
Subject: | Re: [PATCH] Check that index can return in get_actual_variable_range() |
Date: | 2025-09-18 13:36:02 |
Message-ID: | CAHgHdKse6Wq-8EZ-BCyhgZkpqT=aD3pAGkATxJTk5rcVD4fCpQ@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Testing with the src/test/modules/treeb work in the patch series at [1],
modifying treebcanreturn() to always return false and modifying
_treeb_first(), _treeb_next(), and _treeb_endpoint() to set scan->xs_itup
to NULL rather than to a tuple, without the patch there are a number of
test failures with "ERROR: no data returned for index-only scan", but with
the patch applied those errors still appear. They are raised by
nodeIndexonlyscan.c at line 213, inside IndexOnlyNext(), suggesting that
changing treebcanreturn() to return false was not enough to dissuade the
planner from choosing the index for an index only scan.
Is there a bug in how the system selects an index for an index-only scan?
Or is the combination amcanorder=true && amcanreturn=NULL/false not a valid
choice for an index? If the latter, shouldn't there be a check for that
somewhere, or at least an Assert()?
[1]
https://www.postgresql.org/message-id/5083d9ed-837d-47cd-9d40-fded88b62c10%40eisentraut.org
On Thu, Sep 18, 2025 at 5:32 AM Aleksander Alekseev <
aleksander(at)tigerdata(dot)com> wrote:
> Hi Maxime,
>
> > Some recent changes were made to remove the explicit dependency on btree
> indexes in some parts of the code. One of these changes was made in commit
> 9ef1851685b, which allows non-btree indexes to be used in
> get_actual_variable_range(). A follow-up commit ee1ae8b99f9 fixes the cases
> where an index doesn’t have a sortopfamily as this is a prerequisite to be
> used in get_actual_variable_range(). However, I found out recently that
> indices that have ‘amcanorder = true’ but do not allow index-only-scans
> (amcanreturn returns false or is NULL) will pass all of the conditions,
> while they should be rejected since get_actual_variable_range() uses the
> index-only-scan machinery in get_actual_variable_endpoint().
> >
> > Attached is a small patch that adds a check in
> get_actual_variable_range() to reject indices that do not allow index-only
> scans.
>
> Thanks for the patch.
>
> Can you think of any test cases we can add to the code base?
>
> --
> Best regards,
> Aleksander Alekseev
>
--
*Mark Dilger*
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2025-09-18 13:52:03 | Re: Changing shared_buffers without restart |
Previous Message | jian he | 2025-09-18 13:32:58 | Re: someone else to do the list of acknowledgments |