Skip to content

Conversation

@joseph-isaacs
Copy link
Contributor

@joseph-isaacs joseph-isaacs commented Nov 19, 2025

Added an ExprArray and a few reduce rules

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
u
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
u
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
u
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 19, 2025

CodSpeed Performance Report

Merging #5400 will improve performances by 11.24%

Comparing ji/expr-array (0164659) with develop (3df9296)

Summary

⚡ 9 improvements
✅ 1412 untouched
🆕 7 new
⏩ 687 skipped1
🗄️ 28 archived benchmarks run2

Benchmarks breakdown

Benchmark BASE HEAD Change
filter_runend[(1000, 256, 0.005)] 22.2 µs 20 µs +11.24%
filter_runend[(1000, 256, 0.01)] 23.2 µs 20.9 µs +10.75%
filter_runend[(1000, 256, 0.03)] 22.3 µs 20.1 µs +11.19%
take_indices[(1000, 16, 0.005)] 22 µs 19.8 µs +10.6%
take_indices[(1000, 16, 0.01)] 22.1 µs 20 µs +10.37%
take_indices[(1000, 256, 0.005)] 21.7 µs 19.6 µs +10.89%
take_indices[(1000, 256, 0.01)] 21.9 µs 19.8 µs +10.61%
take_indices[(1000, 256, 0.03)] 22.3 µs 20.2 µs +10.4%
take_indices[(1000, 4, 0.01)] 23.3 µs 21.1 µs +10.22%
🆕 decompress[("alp_for_bp_f64", 0x4658bf0)] N/A 24.2 ms N/A
🆕 decompress[("datetime_for_bp", 0x465bab0)] N/A 34.9 ms N/A
🆕 decompress[("dict_fsst_varbin_bp_string", 0x465adf0)] N/A 14.5 ms N/A
🆕 decompress[("dict_fsst_varbin_string", 0x465a950)] N/A 14.5 ms N/A
🆕 decompress[("dict_varbinview_string", 0x4659610)] N/A 14.7 ms N/A
🆕 decompress[("for_bp_u64", 0x46584a0)] N/A 2.5 ms N/A
🆕 decompress[("runend_for_bp_u32", 0x4659aa0)] N/A 2 ms N/A

Footnotes

  1. 687 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.

  2. 28 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them.

@codecov
Copy link

codecov bot commented Nov 19, 2025

Codecov Report

❌ Patch coverage is 82.23235% with 78 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.93%. Comparing base (dc03c4f) to head (0164659).
⚠️ Report is 7 commits behind head on develop.

Files with missing lines Patch % Lines
vortex-array/src/arrays/expr/vtable/mod.rs 0.00% 33 Missing ⚠️
vortex-array/src/arrays/expr/vtable/operations.rs 0.00% 18 Missing ⚠️
vortex-array/src/arrays/expr/vtable/array.rs 47.36% 10 Missing ⚠️
vortex-array/src/arrays/struct_/vtable/reduce.rs 96.77% 5 Missing ⚠️
vortex-array/src/arrays/expr/vtable/operator.rs 80.95% 4 Missing ⚠️
vortex-array/src/arrays/expr/vtable/visitor.rs 0.00% 4 Missing ⚠️
vortex-array/src/arrays/struct_/vtable/operator.rs 97.95% 3 Missing ⚠️
vortex-array/src/arrays/expr/array.rs 96.15% 1 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

@joseph-isaacs joseph-isaacs enabled auto-merge (squash) November 19, 2025 15:57
@joseph-isaacs joseph-isaacs enabled auto-merge (squash) November 19, 2025 15:57
u
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
@joseph-isaacs joseph-isaacs requested review from 0ax1 and gatesn November 19, 2025 16:17
@joseph-isaacs
Copy link
Contributor Author

@claude review pelase

u
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
@vortex-data vortex-data deleted a comment from claude bot Nov 19, 2025
// Check if the parent is an ExprArray wrapping this struct
// If so, we can partition the expression over the struct fields
if let Some(expr_array) = parent.as_opt::<ExprVTable>() {
let session = ExprSession::default();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a plan of how to inject this in a future PR using the expr session context

/// The caller must ensure that `dtype` matches `expr.return_dtype(child.dtype())`.
/// Violating this invariant may lead to incorrect results or panics when the array is used.
pub unsafe fn unchecked_new(child: ArrayRef, expr: Expression, dtype: DType) -> Self {
Self {
Copy link
Contributor

Choose a reason for hiding this comment

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

I quite like the pattern we have started to use in vortex-vector where we have new that calls try_new().expect(), and new_unchecked (note not unchecked_new) has if cfg(debug_assertions) { try_new().expect() } else { Self { ... }}

impl OperatorVTable<ExprVTable> for ExprVTable {
fn reduce(array: &ExprArray) -> VortexResult<Option<ArrayRef>> {
// Get the default expression session
let session = ExprSession::default();
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe leave a louder fixme / todo that we have to inject this

@joseph-isaacs joseph-isaacs merged commit 50ddc86 into develop Nov 19, 2025
40 checks passed
@joseph-isaacs joseph-isaacs deleted the ji/expr-array branch November 19, 2025 19:38
joseph-isaacs added a commit that referenced this pull request Nov 20, 2025
Added an ExprArray and a few reduce rules

----

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
a10y pushed a commit that referenced this pull request Nov 25, 2025
Added an ExprArray and a few reduce rules

----

Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
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.

3 participants