Skip to content

fix(encoding): reject stalled RLE miniblock encoding#7729

Merged
Xuanwo merged 1 commit into
lance-format:mainfrom
Ali2Arslan:fix/rle-zero-progress
Jul 15, 2026
Merged

fix(encoding): reject stalled RLE miniblock encoding#7729
Xuanwo merged 1 commit into
lance-format:mainfrom
Ali2Arslan:fix/rle-zero-progress

Conversation

@Ali2Arslan

@Ali2Arslan Ali2Arslan commented Jul 10, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

  • fail RLE miniblock encoding when chunk selection cannot advance instead of returning partial buffers with the original value count
  • assert that raw encoded run lengths match each declared chunk count across U8, U16, and U32 run-length widths
  • retain the exact-prefix implementation introduced in feat: add RLE v2 run length widths #7376; this does not change the file format

Context

LANCE_MINIBLOCK_MAX_VALUES=1 is currently accepted, but a multi-value RLE page cannot represent a non-final one-value chunk because log_num_values == 0 identifies the final chunk. The encoder previously broke out of the loop and returned incomplete data while retaining the original num_values.

Test plan

  • cargo test -p lance-encoding (430 passed, 5 ignored)
  • cargo fmt --all -- --check
  • cargo clippy --all --tests --benches -- -D warnings

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes

    • Prevented incomplete or invalid compressed data from being returned when encoding cannot make progress.
    • Improved error reporting with contextual details for encoding failures.
    • Fixed run-length handling across the 2,048-value boundary.

  • Tests

    • Added coverage for multiple run-length widths and zero-progress encoding scenarios.

@Ali2Arslan @cursoragent
fix(encoding): reject stalled RLE miniblock encoding
742640e 
Return an error instead of emitting partial buffers when chunk selection cannot advance, and pin raw run-count invariants across all RLE widths.

Co-authored-by: Cursor <[email protected]>
@github-actions github-actions Bot added A-encoding Encoding, IO, file reader/writer bug Something isn't working labels Jul 10, 2026
@coderabbitai

coderabbitai Bot commented Jul 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f9f5450b-d680-48f7-a6a2-888c25913a38

📥 Commits

Reviewing files that changed from the base of the PR and between 1aec146 and 742640e.

📒 Files selected for processing (1)
  • rust/lance-encoding/src/encodings/physical/rle.rs
📝 Walkthrough

Walkthrough

The RLE encoder now returns an internal error when miniblock encoding makes zero progress. Tests cover run-length consistency across the 2,048-value boundary for all supported widths and verify the error context.

Changes

RLE encoding

Layer / File(s) Summary
Reject zero-progress encoding
rust/lance-encoding/src/encodings/physical/rle.rs		 encode_data reports an internal error with remaining-value and encoding-context details when a miniblock iteration processes zero values.
Validate chunk lengths and errors
rust/lance-encoding/src/encodings/physical/rle.rs		 Parameterized tests verify encoded run lengths for U8, U16, and U32 across chunk boundaries, while a targeted test checks the zero-progress error message.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: rejecting stalled RLE miniblock encoding.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-encoding/src/encodings/physical/rle.rs 94.44% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@westonpace
westonpace requested a review from Xuanwo July 10, 2026 22:49
Xuanwo
Xuanwo approved these changes Jul 15, 2026
View reviewed changes

@Xuanwo Xuanwo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you!

@Xuanwo
Xuanwo merged commit 362cd5b into lance-format:main Jul 15, 2026
35 checks passed
@Ali2Arslan
Ali2Arslan deleted the fix/rle-zero-progress branch July 22, 2026 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

Assignees

No one assigned

Labels

A-encoding Encoding, IO, file reader/writer bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Ali2Arslan @Xuanwo