fix: take_blobs_by_indices fails with stable row IDs on fragment 1+ by jmhsieh · Pull Request #5392 · lance-format/lance

jmhsieh · 2025-12-02T21:18:11Z

When enable_stable_row_ids=true, take_blobs_by_indices was failing with "index out of bounds" for rows in fragment 1+. The bug was caused by passing row addresses to blob::take_blobs which expected row IDs.

Root cause:

take_blobs_by_indices converts indices to row addresses
It passed these addresses to take_blobs which calls take_builder
TakeBuilder.get_row_addrs() looked up the values in the row ID index
For fragment 0: addresses (0,1,2) matched row IDs (0,1,2) by accident
For fragment 1+: addresses (4294967296+) didn't match any row IDs
This caused empty results and missing _rowaddr column → panic

Fix:

Add take_blobs_by_addresses() that uses TakeBuilder::try_new_from_addresses to bypass the row ID index lookup
Update take_blobs_by_indices to call the new function
Add defensive fix in do_take_rows to include _rowaddr column in empty batches

🤖 Generated with Claude Code

When `enable_stable_row_ids=true`, `take_blobs_by_indices` was failing with "index out of bounds" for rows in fragment 1+. The bug was caused by passing row addresses to `blob::take_blobs` which expected row IDs. Root cause: - `take_blobs_by_indices` converts indices to row addresses - It passed these addresses to `take_blobs` which calls `take_builder` - `TakeBuilder.get_row_addrs()` looked up the values in the row ID index - For fragment 0: addresses (0,1,2) matched row IDs (0,1,2) by accident - For fragment 1+: addresses (4294967296+) didn't match any row IDs - This caused empty results and missing `_rowaddr` column → panic Fix: - Add `take_blobs_by_addresses()` that uses `TakeBuilder::try_new_from_addresses` to bypass the row ID index lookup - Update `take_blobs_by_indices` to call the new function - Add defensive fix in `do_take_rows` to include `_rowaddr` column in empty batches 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

codecov · 2025-12-02T21:44:26Z

Codecov Report

❌ Patch coverage is 68.88889% with 28 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
rust/lance/src/dataset/blob.rs	78.37%	7 Missing and 9 partials ⚠️
rust/lance/src/dataset.rs	12.50%	7 Missing ⚠️
rust/lance/src/dataset/take.rs	37.50%	4 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

Address PR feedback: - Make take_blobs_by_addresses public (was pub(super)) - Add Dataset::take_blobs_by_addresses public method - Simplify take_blobs_by_indices doc to remove internal details - Add proper public documentation for take_blobs_by_addresses This allows callers to use row addresses directly when they already have them, providing flexibility alongside take_blobs (for row IDs) and take_blobs_by_indices (for row indices/offsets). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

chatgpt-codex-connector · 2025-12-03T04:23:00Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

jackye1995

looks good to me!

jackye1995 · 2025-12-03T18:33:16Z

@jmhsieh looks like there is a merge conflict, could you resolve?

# Conflicts: # rust/lance/src/dataset/blob.rs

jmhsieh · 2025-12-03T18:48:48Z

@jmhsieh looks like there is a merge conflict, could you resolve?

updated and giving a go

🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

…ance-format#5392) When `enable_stable_row_ids=true`, `take_blobs_by_indices` was failing with "index out of bounds" for rows in fragment 1+. The bug was caused by passing row addresses to `blob::take_blobs` which expected row IDs. Root cause: - `take_blobs_by_indices` converts indices to row addresses - It passed these addresses to `take_blobs` which calls `take_builder` - `TakeBuilder.get_row_addrs()` looked up the values in the row ID index - For fragment 0: addresses (0,1,2) matched row IDs (0,1,2) by accident - For fragment 1+: addresses (4294967296+) didn't match any row IDs - This caused empty results and missing `_rowaddr` column → panic Fix: - Add `take_blobs_by_addresses()` that uses `TakeBuilder::try_new_from_addresses` to bypass the row ID index lookup - Update `take_blobs_by_indices` to call the new function - Add defensive fix in `do_take_rows` to include `_rowaddr` column in empty batches 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>

…5392) When `enable_stable_row_ids=true`, `take_blobs_by_indices` was failing with "index out of bounds" for rows in fragment 1+. The bug was caused by passing row addresses to `blob::take_blobs` which expected row IDs. Root cause: - `take_blobs_by_indices` converts indices to row addresses - It passed these addresses to `take_blobs` which calls `take_builder` - `TakeBuilder.get_row_addrs()` looked up the values in the row ID index - For fragment 0: addresses (0,1,2) matched row IDs (0,1,2) by accident - For fragment 1+: addresses (4294967296+) didn't match any row IDs - This caused empty results and missing `_rowaddr` column → panic Fix: - Add `take_blobs_by_addresses()` that uses `TakeBuilder::try_new_from_addresses` to bypass the row ID index lookup - Update `take_blobs_by_indices` to call the new function - Add defensive fix in `do_take_rows` to include `_rowaddr` column in empty batches 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>

…ance-format#5392) When `enable_stable_row_ids=true`, `take_blobs_by_indices` was failing with "index out of bounds" for rows in fragment 1+. The bug was caused by passing row addresses to `blob::take_blobs` which expected row IDs. Root cause: - `take_blobs_by_indices` converts indices to row addresses - It passed these addresses to `take_blobs` which calls `take_builder` - `TakeBuilder.get_row_addrs()` looked up the values in the row ID index - For fragment 0: addresses (0,1,2) matched row IDs (0,1,2) by accident - For fragment 1+: addresses (4294967296+) didn't match any row IDs - This caused empty results and missing `_rowaddr` column → panic Fix: - Add `take_blobs_by_addresses()` that uses `TakeBuilder::try_new_from_addresses` to bypass the row ID index lookup - Update `take_blobs_by_indices` to call the new function - Add defensive fix in `do_take_rows` to include `_rowaddr` column in empty batches 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>

github-actions Bot added the bug Something isn't working label Dec 2, 2025

style: apply cargo fmt

a8a6705

🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

jackye1995 reviewed Dec 3, 2025

View reviewed changes

Comment thread rust/lance/src/dataset.rs Outdated

jackye1995 reviewed Dec 3, 2025

View reviewed changes

Comment thread rust/lance/src/dataset/blob.rs Outdated

jmhsieh and others added 2 commits December 2, 2025 19:08

Merge branch 'main' into jmhsieh/fix-stable_row_id-blobs

2e59672

jmhsieh marked this pull request as ready for review December 3, 2025 04:22

jmhsieh requested a review from jackye1995 December 3, 2025 04:22

jackye1995 approved these changes Dec 3, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into HEAD

a7e20f7

# Conflicts: # rust/lance/src/dataset/blob.rs

style: apply cargo fmt

81c1530

🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

jackye1995 merged commit 88c7fcb into lance-format:main Dec 4, 2025
26 checks passed

andrea-reale mentioned this pull request Mar 30, 2026

emilk/fix write starvation rerun-io/lance#12

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: take_blobs_by_indices fails with stable row IDs on fragment 1+#5392

fix: take_blobs_by_indices fails with stable row IDs on fragment 1+#5392
jackye1995 merged 6 commits intolance-format:mainfrom
jmhsieh:jmhsieh/fix-stable_row_id-blobs

jmhsieh commented Dec 2, 2025

Uh oh!

codecov Bot commented Dec 2, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Dec 3, 2025

Uh oh!

jackye1995 left a comment

Uh oh!

jackye1995 commented Dec 3, 2025

Uh oh!

jmhsieh commented Dec 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jmhsieh commented Dec 2, 2025

Uh oh!

codecov Bot commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Dec 3, 2025

Uh oh!

jackye1995 left a comment

Choose a reason for hiding this comment

Uh oh!

jackye1995 commented Dec 3, 2025

Uh oh!

jmhsieh commented Dec 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Dec 2, 2025 •

edited

Loading