Skip to content

Conversation

@knizhnik
Copy link

No description provided.

Comment on lines 893 to 894
else if (!BlockNumberIsValid(so->next_parent))
elog(FATAL, "btpo_next is invalid");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opaque->ptpo_next can be P_NONE, but IIUC that is 0, not InvalidBlockNumber. I don't understand how it could be InvalidBlockNumber.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the check isn't wrong per se, I think this is irrelevant as btpo_prev shouldn't be InvalidBlockNumber, ever.

Same comment on the same addition around line 908-912 - I think both can be reverted.

@knizhnik knizhnik force-pushed the ios_prefetch_fix_v17 branch from 4fc8da9 to 424b5ba Compare December 1, 2024 15:59
Copy link
Contributor

@MMeent MMeent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an improvement over the previous version, but seems to have some additions that I'd rather see gone.

Comment on lines 893 to 894
else if (!BlockNumberIsValid(so->next_parent))
elog(FATAL, "btpo_next is invalid");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the check isn't wrong per se, I think this is irrelevant as btpo_prev shouldn't be InvalidBlockNumber, ever.

Same comment on the same addition around line 908-912 - I think both can be reverted.

@knizhnik knizhnik force-pushed the ios_prefetch_fix_v17 branch from 398fb77 to 9b1bcff Compare December 12, 2024 06:00
@knizhnik knizhnik requested a review from MMeent December 13, 2024 15:06
@knizhnik knizhnik merged commit 010c0ea into REL_17_STABLE_neon Dec 13, 2024
1 check passed
@knizhnik knizhnik deleted the ios_prefetch_fix_v17 branch December 13, 2024 16:30
github-merge-queue bot pushed a commit to neondatabase/neon that referenced this pull request Dec 13, 2024
…index-only scan (#9867)

## Problem

See #9866

Index-only scan prefetch implementation doesn't take in account that
down link may be invalid

## Summary of changes

Check that downlink is valid block number


Correspondent Postgres PRs:
neondatabase/postgres#534
neondatabase/postgres#535
neondatabase/postgres#536
neondatabase/postgres#537

---------

Co-authored-by: Konstantin Knizhnik <knizhnik@neon.tech>
lubennikovaav pushed a commit that referenced this pull request May 9, 2025
…index-only scan (#534)

* Check for invalid down link while prefetching B-Tree leave pages for index-only scan

* Replace assert which doesn't take in account presence of invalid downlinks

* Replace assert which doesn't take in account presence of invalid downlinks

* Add extra checks for IOS prefetch

* Do not use stack->bts_offset because content of parent page can be changed

* Remove unintended changes

---------

Co-authored-by: Konstantin Knizhnik <knizhnik@neon.tech>
MMeent pushed a commit that referenced this pull request Jul 3, 2025
…index-only scan (#534)

* Check for invalid down link while prefetching B-Tree leave pages for index-only scan

* Replace assert which doesn't take in account presence of invalid downlinks

* Replace assert which doesn't take in account presence of invalid downlinks

* Add extra checks for IOS prefetch

* Do not use stack->bts_offset because content of parent page can be changed

* Remove unintended changes

---------

Co-authored-by: Konstantin Knizhnik <knizhnik@neon.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants