Skip to content

feat(array): cast bool and primitive values to Utf8 - #9261

Open
haohuaijin wants to merge 7 commits into
vortex-data:developfrom
haohuaijin:codex/bool-primitive-to-utf8
Open

feat(array): cast bool and primitive values to Utf8#9261
haohuaijin wants to merge 7 commits into
vortex-data:developfrom
haohuaijin:codex/bool-primitive-to-utf8

Conversation

@haohuaijin

@haohuaijin haohuaijin commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

DataFusion can push CAST(... AS STRING) into Vortex scans, but boolean and primitive arrays currently lack native casts to Utf8, causing pushed queries to fail with a missing CastKernel.

Related to #6211, #8621, and #6702.

What changes are included in this PR?

  • Add BoolArray -> Utf8 and PrimitiveArray -> Utf8 casts.
  • Add matching scalar casts for constant and nested values. This includes Bool -> Primitive so they behave consistently with the existing BoolArray -> Primitive cast (true = 1, false = 0).
  • Preserve nullability and use Arrow-compatible numeric formatting.
  • Add unit tests and a DataFusion projection-pushdown E2E test.

Validation:

  • cargo nextest run -p vortex-array
  • cargo nextest run -p vortex-datafusion
    • Includes an E2E test that writes nullable Boolean, Int64, and Float64 columns to a Vortex file, runs CAST(... AS STRING) with projection pushdown enabled, and verifies both values and nulls.
    • The E2E test fails on develop with a missing Bool -> Utf8 cast kernel and passes with this PR.
  • cargo clippy -p vortex-datafusion --all-targets --all-features
  • cargo +nightly fmt --all

What APIs are changed? Are there any user-facing changes?

No public API signatures are changed.

Boolean and primitive values can now be cast to Utf8 inside Vortex, including DataFusion projection pushdown.

@haohuaijin
haohuaijin force-pushed the codex/bool-primitive-to-utf8 branch from 36c6f4f to ff0d6f9 Compare August 7, 2026 06:19
@haohuaijin
haohuaijin marked this pull request as ready for review August 7, 2026 07:39
Signed-off-by: Huaijin <haohuaijin@gmail.com>
…r path

Signed-off-by: Huaijin <haohuaijin@gmail.com>
Signed-off-by: Huaijin <haohuaijin@gmail.com>
Drop the TestResult alias and inline crate::Canonical qualifications in the
primitive cast tests, move to_bit_buffer out of the all-null bool cast path,
and cover f16 NaN/inf formatting in the array-path utf8 cast.

Signed-off-by: Huaijin <haohuaijin@gmail.com>
@haohuaijin
haohuaijin force-pushed the codex/bool-primitive-to-utf8 branch from db1e564 to d12b54a Compare August 8, 2026 02:42
@haohuaijin

Copy link
Copy Markdown
Contributor Author

hi @AdamGS , do you have time take a look?

Comment thread vortex-array/src/arrays/bool/compute/cast.rs
Comment thread vortex-array/src/arrays/constant/compute/cast.rs
@codspeed-hq

codspeed-hq Bot commented Aug 11, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 2.12%

⚡ 2 improved benchmarks
❌ 2 regressed benchmarks
✅ 1958 untouched benchmarks
⏩ 89 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation search_index_in_range_chunked 5.4 ms 6.4 ms -15.17%
Simulation search_index_above_max_chunked 4.7 ms 5.4 ms -12.33%
Simulation chunked_dict_primitive_canonical_into[u32, (1000, 1000, 100)] 2 ms 1.8 ms +11.33%
Simulation chunked_dict_primitive_canonical_into[u32, (1000, 10, 100)] 1.6 ms 1.4 ms +10.84%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing haohuaijin:codex/bool-primitive-to-utf8 (2c3825c) with develop (c4bb934)

Open in CodSpeed

Footnotes

  1. 89 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please use generics and a macro each macro (we use elsewhere)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why did you need the new trait where did the other definitions come from where could we place this code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is for unify the float to utf8 convert to use append_float_values_to_utf8 func

fn append_float_values_to_utf8<T>(builder: &mut VarBinViewBuilder, values: &[T], mask: &Mask)
where
    T: FloatToUtf8,
{
    let mut formatter = T::formatter();
    append_values_to_utf8(builder, values, mask, |builder, value| {
        builder.append_value(value.format(&mut formatter));
    });
}

i follower the arrow-rs, float32/float64 use ryu, float16 use display, https://github.com/apache/arrow-rs/blob/fc16607ccfdb298557ed6630742941761ac18ce3/arrow-cast/src/display.rs#L711-L734

Comment thread vortex-array/src/arrays/primitive/compute/cast.rs Outdated
@joseph-isaacs

Copy link
Copy Markdown
Contributor

Why did you add bool -> prim here too?

Signed-off-by: Huaijin <haohuaijin@gmail.com>
@haohuaijin

haohuaijin commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for your reviews @joseph-isaacs , i apply you suggestion in 2c3825c.

Why did you add bool -> prim here too?

sorry make you misunderstand, the pr #8621 impl the bool -> prim cast, but only for BoolArray, so in this pr i also add it for BoolScalar. should i remove it?

Comment on lines +71 to +80
let views = if true_count <= len - true_count {
let mut views = BufferMut::full(false_view, len);
values.for_each_set_index(|index| views[index] = true_view);
views
} else {
let mut views = BufferMut::full(true_view, len);
(!&values).for_each_set_index(|index| views[index] = false_view);
views
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you explain the trick

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

first init the full buffer with more frequent value(true/false), then only overwrite the another(false/true) positions, that minimize the index write.

@joseph-isaacs

Copy link
Copy Markdown
Contributor

This new a few comments explain the complex/interesting stuff and thoughts about where to put new trait or can we extend existing ones?

@joseph-isaacs joseph-isaacs added the changelog/feature A new feature label Aug 11, 2026
@joseph-isaacs

Copy link
Copy Markdown
Contributor

please do split

Signed-off-by: Huaijin <haohuaijin@gmail.com>
@haohuaijin

Copy link
Copy Markdown
Contributor Author

please do split

split it out of this pr

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

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants