Skip to content

fix: inconsistency inline constant handling for non‑fixed‑stride datatype#5851

Closed
zhangyue19921010 wants to merge 8 commits intolance-format:mainfrom
zhangyue19921010:fix-inconsistency
Closed

fix: inconsistency inline constant handling for non‑fixed‑stride datatype#5851
zhangyue19921010 wants to merge 8 commits intolance-format:mainfrom
zhangyue19921010:fix-inconsistency

Conversation

@zhangyue19921010
Copy link
Copy Markdown
Contributor

Encoding side (rust/lance-arrow/src/scalar.rs)

try_inline_value decides whether a scalar value can be written inline. The old logic only checked that:

  • the scalar is non-null and a single value
  • it has no child data
  • there is only one buffer
  • the inline size does not exceed the threshold

However, it did not verify that the data type is fixed-stride or not. TAKE BOOLEAN AS EXAMPLE

Decoding side (decode_scalar_from_inline_value)

Decoding explicitly requires data_type.byte_width_opt() to be present; otherwise it errors out:

  • Inline constant is not supported for non-fixed-stride data type Boolean.

This creates an inconsistency between the encoder and decoder.

Without this fix, new added unit test test_constant_layout_out_of_line_boolean_v2_2 and similar scenarios will report errors

called `Result::unwrap()` on an `Err` value: Arrow { message: "Invalid argument error: 
Inline constant is not supported for non-fixed-stride data type Boolean", 
location: Location { file: "rust/lance-encoding/src/encodings/logical/primitive/constant.rs", line: 246, column: 59 } }

@github-actions github-actions Bot added the bug Something isn't working label Jan 29, 2026
@zhangyue19921010
Copy link
Copy Markdown
Contributor Author

Hi @Xuanwo , would u mind to take a look at your convenient? Thanks!

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 29, 2026

Codecov Report

❌ Patch coverage is 91.30435% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../lance-encoding/src/encodings/logical/primitive.rs 89.47% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@wjones127 wjones127 requested a review from Xuanwo February 12, 2026 01:01
Copy link
Copy Markdown
Contributor

@wjones127 wjones127 left a comment

Choose a reason for hiding this comment

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

This seems reasonable, but would like @Xuanwo to take a look before we merge.

Comment thread rust/lance-arrow/src/scalar.rs Outdated
@zhangyue19921010
Copy link
Copy Markdown
Contributor Author

Thanks for your help @wjones127 changed.

Copy link
Copy Markdown
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

One minor nit and then we can merge. Just ping me when you've addressed it.


let test_cases = TestCases::default()
.with_min_file_version(LanceFileVersion::V2_2)
.with_max_file_version(LanceFileVersion::V2_2)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we remove the with_max_file_version? There is no reason this test should stop working in future versions.

@zhangyue19921010
Copy link
Copy Markdown
Contributor Author

This Bug is fixed by another PR #5862

Thanks for your attention and help here @westonpace and @wjones127 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants