Skip to content

Fix: skip boolean column when all values are false for optional arrays#2759

Open
marcsnid wants to merge 1 commit intoopen-telemetry:mainfrom
marcsnid:marcsnid/fix-boolean-column-default-handling
Open

Fix: skip boolean column when all values are false for optional arrays#2759
marcsnid wants to merge 1 commit intoopen-telemetry:mainfrom
marcsnid:marcsnid/fix-boolean-column-default-handling

Conversation

@marcsnid
Copy link
Copy Markdown
Contributor

Change Summary

Fix AdaptiveBooleanArrayBuilder to skip producing an array when all values are false or null for optional columns. Previously, the builder only skipped on all-null:false values triggered array creation even though false is the boolean default value. This makes boolean columns consistent with how integer (0) and string ("") columns handle defaults.

Changes:

  • Added has_true_value and optional fields to AdaptiveBooleanArrayBuilder
  • finish() now returns None when optional and !has_true_value
  • Updated test_metrics_round_trip to reflect that the is_monotonic column (all false/null) is now correctly omitted

What issue does this PR close?

#1449

How are these changes tested?

  • Added 4 new unit tests in boolean.rs:
    • test_adaptive_boolean_builder_all_false — all-false optional → None
    • test_adaptive_boolean_builder_false_and_null — mixed false+null optional → None
    • test_adaptive_boolean_builder_false_then_true — false then true → Some(array) with correct values
    • test_adaptive_boolean_builder_all_false_non_optional — non-optional always produces array
  • All existing tests in otap-df-pdata continue to pass (including the updated test_metrics_round_trip).

Are there any user-facing changes?

No. This is an internal encoding optimization. Optional boolean columns that contain only default values (false/null) are no longer included in Arrow record batches, reducing payload size.

@marcsnid marcsnid requested a review from a team as a code owner April 26, 2026 01:38
@github-actions github-actions Bot added the rust Pull requests that update Rust code label Apr 26, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.24%. Comparing base (993ba07) to head (78256ef).
⚠️ Report is 3 commits behind head on main.

❌ Your project check has failed because the head coverage (82.24%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (993ba07) and HEAD (78256ef). Click for more details.

HEAD has 5 uploads less than BASE
Flag BASE (993ba07) HEAD (78256ef)
8 3
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2759       +/-   ##
===========================================
- Coverage   88.06%   82.24%    -5.83%     
===========================================
  Files         644      181      -463     
  Lines      246791    52744   -194047     
===========================================
- Hits       217340    43377   -173963     
+ Misses      28927     8843    -20084     
  Partials      524      524               
Components Coverage Δ
otap-dataflow ∅ <ø> (∅)
query_abstraction 80.61% <ø> (ø)
query_engine 90.75% <ø> (ø)
otel-arrow-go 52.45% <ø> (ø)
quiver ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

rust Pull requests that update Rust code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant