Skip to content

Commit 91f1c2f

Browse files
authored
Remove ScalarFnConstantRule (#7575)
This reduction rule does execution and prevents any parent reduce rules from triggering --------- Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent be2a14b commit 91f1c2f

3 files changed

Lines changed: 22 additions & 46 deletions

File tree

vortex-array/src/arrays/scalar_fn/rules.rs

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@ use vortex_error::VortexExpect;
99
use vortex_error::VortexResult;
1010

1111
use crate::ArrayRef;
12-
use crate::Canonical;
1312
use crate::IntoArray;
14-
use crate::LEGACY_SESSION;
15-
use crate::VortexSessionExecute;
1613
use crate::array::ArrayView;
1714
use crate::arrays::Constant;
1815
use crate::arrays::ConstantArray;
@@ -34,11 +31,8 @@ use crate::scalar_fn::ScalarFnRef;
3431
use crate::scalar_fn::fns::pack::Pack;
3532
use crate::validity::Validity;
3633

37-
pub(super) const RULES: ReduceRuleSet<ScalarFnVTable> = ReduceRuleSet::new(&[
38-
&ScalarFnPackToStructRule,
39-
&ScalarFnConstantRule,
40-
&ScalarFnAbstractReduceRule,
41-
]);
34+
pub(super) const RULES: ReduceRuleSet<ScalarFnVTable> =
35+
ReduceRuleSet::new(&[&ScalarFnPackToStructRule, &ScalarFnAbstractReduceRule]);
4236

4337
pub(super) const PARENT_RULES: ParentRuleSet<ScalarFnVTable> = ParentRuleSet::new(&[
4438
ParentRuleSet::lift(&ScalarFnUnaryFilterPushDownRule),
@@ -71,24 +65,6 @@ impl ArrayReduceRule<ScalarFnVTable> for ScalarFnPackToStructRule {
7165
}
7266
}
7367

74-
#[derive(Debug)]
75-
struct ScalarFnConstantRule;
76-
impl ArrayReduceRule<ScalarFnVTable> for ScalarFnConstantRule {
77-
fn reduce(&self, array: ArrayView<'_, ScalarFnVTable>) -> VortexResult<Option<ArrayRef>> {
78-
if !array.children().iter().all(|c| c.is::<Constant>()) {
79-
return Ok(None);
80-
}
81-
if array.is_empty() {
82-
Ok(Some(Canonical::empty(array.dtype()).into_array()))
83-
} else {
84-
let result = array
85-
.array()
86-
.execute_scalar(0, &mut LEGACY_SESSION.create_execution_ctx())?;
87-
Ok(Some(ConstantArray::new(result, array.len()).into_array()))
88-
}
89-
}
90-
}
91-
9268
#[derive(Debug)]
9369
struct ScalarFnSliceReduceRule;
9470
impl ArrayParentReduceRule<ScalarFnVTable> for ScalarFnSliceReduceRule {

vortex-array/src/scalar_fn/fns/between/mod.rs

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -334,14 +334,13 @@ impl ScalarFnVTable for Between {
334334

335335
#[cfg(test)]
336336
mod tests {
337+
use std::sync::LazyLock;
338+
337339
use rstest::rstest;
338340
use vortex_buffer::buffer;
339341

340342
use super::*;
341343
use crate::IntoArray;
342-
use crate::LEGACY_SESSION;
343-
#[expect(deprecated)]
344-
use crate::ToCanonical as _;
345344
use crate::VortexSessionExecute;
346345
use crate::arrays::BoolArray;
347346
use crate::arrays::DecimalArray;
@@ -356,9 +355,13 @@ mod tests {
356355
use crate::expr::root;
357356
use crate::scalar::DecimalValue;
358357
use crate::scalar::Scalar;
358+
use crate::session::ArraySession;
359359
use crate::test_harness::to_int_indices;
360360
use crate::validity::Validity;
361361

362+
static SESSION: LazyLock<VortexSession> =
363+
LazyLock::new(|| VortexSession::empty().with::<ArraySession>());
364+
362365
#[test]
363366
fn test_display() {
364367
let expr = between(
@@ -398,7 +401,6 @@ mod tests {
398401
let array = buffer![1, 0, 1, 0, 1].into_array();
399402
let upper = buffer![2, 1, 1, 0, 0].into_array();
400403

401-
#[expect(deprecated)]
402404
let matches = between_canonical(
403405
&array,
404406
&lower,
@@ -407,10 +409,11 @@ mod tests {
407409
lower_strict,
408410
upper_strict,
409411
},
410-
&mut LEGACY_SESSION.create_execution_ctx(),
412+
&mut SESSION.create_execution_ctx(),
411413
)
412414
.unwrap()
413-
.to_bool();
415+
.execute::<BoolArray>(&mut SESSION.create_execution_ctx())
416+
.unwrap();
414417

415418
let indices = to_int_indices(matches).unwrap();
416419
assert_eq!(indices, expected);
@@ -428,7 +431,6 @@ mod tests {
428431
)
429432
.into_array();
430433

431-
#[expect(deprecated)]
432434
let matches = between_canonical(
433435
&array,
434436
&lower,
@@ -437,17 +439,17 @@ mod tests {
437439
lower_strict: StrictComparison::NonStrict,
438440
upper_strict: StrictComparison::NonStrict,
439441
},
440-
&mut LEGACY_SESSION.create_execution_ctx(),
442+
&mut SESSION.create_execution_ctx(),
441443
)
442444
.unwrap()
443-
.to_bool();
445+
.execute::<BoolArray>(&mut SESSION.create_execution_ctx())
446+
.unwrap();
444447

445448
let indices = to_int_indices(matches).unwrap();
446449
assert!(indices.is_empty());
447450

448451
// upper is a fixed constant
449452
let upper = ConstantArray::new(Scalar::from(2), 5).into_array();
450-
#[expect(deprecated)]
451453
let matches = between_canonical(
452454
&array,
453455
&lower,
@@ -456,17 +458,17 @@ mod tests {
456458
lower_strict: StrictComparison::NonStrict,
457459
upper_strict: StrictComparison::NonStrict,
458460
},
459-
&mut LEGACY_SESSION.create_execution_ctx(),
461+
&mut SESSION.create_execution_ctx(),
460462
)
461463
.unwrap()
462-
.to_bool();
464+
.execute::<BoolArray>(&mut SESSION.create_execution_ctx())
465+
.unwrap();
463466
let indices = to_int_indices(matches).unwrap();
464467
assert_eq!(indices, vec![0, 1, 3]);
465468

466469
// lower is also a constant
467470
let lower = ConstantArray::new(Scalar::from(0), 5).into_array();
468471

469-
#[expect(deprecated)]
470472
let matches = between_canonical(
471473
&array,
472474
&lower,
@@ -475,10 +477,11 @@ mod tests {
475477
lower_strict: StrictComparison::NonStrict,
476478
upper_strict: StrictComparison::NonStrict,
477479
},
478-
&mut LEGACY_SESSION.create_execution_ctx(),
480+
&mut SESSION.create_execution_ctx(),
479481
)
480482
.unwrap()
481-
.to_bool();
483+
.execute::<BoolArray>(&mut SESSION.create_execution_ctx())
484+
.unwrap();
482485
let indices = to_int_indices(matches).unwrap();
483486
assert_eq!(indices, vec![0, 1, 2, 3, 4]);
484487
}
@@ -517,7 +520,7 @@ mod tests {
517520
lower_strict: StrictComparison::Strict,
518521
upper_strict: StrictComparison::NonStrict,
519522
},
520-
&mut LEGACY_SESSION.create_execution_ctx(),
523+
&mut SESSION.create_execution_ctx(),
521524
)
522525
.unwrap();
523526
assert_arrays_eq!(
@@ -534,7 +537,7 @@ mod tests {
534537
lower_strict: StrictComparison::NonStrict,
535538
upper_strict: StrictComparison::Strict,
536539
},
537-
&mut LEGACY_SESSION.create_execution_ctx(),
540+
&mut SESSION.create_execution_ctx(),
538541
)
539542
.unwrap();
540543
assert_arrays_eq!(

vortex-array/src/scalar_fn/fns/list_contains/mod.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,6 @@ mod tests {
483483
use crate::expr::stats::Stat;
484484
use crate::scalar::Scalar;
485485
use crate::scalar_fn::fns::list_contains::BoolArray;
486-
use crate::scalar_fn::fns::list_contains::Constant;
487486
use crate::scalar_fn::fns::list_contains::ConstantArray;
488487
use crate::scalar_fn::fns::list_contains::ListViewArray;
489488
use crate::scalar_fn::fns::list_contains::PrimitiveArray;
@@ -827,7 +826,6 @@ mod tests {
827826

828827
let expr = list_contains(root(), lit(2i32));
829828
let contains = list_array.apply(&expr).unwrap();
830-
assert!(contains.is::<Constant>(), "Expected constant result");
831829
let expected = BoolArray::from_iter([true, true]);
832830
assert_arrays_eq!(contains, expected);
833831
}
@@ -845,7 +843,6 @@ mod tests {
845843

846844
let expr = list_contains(root(), lit(2i32));
847845
let contains = list_array.apply(&expr).unwrap();
848-
assert!(contains.is::<Constant>(), "Expected constant result");
849846

850847
let expected = BoolArray::new(
851848
[false, false, false, false, false].into_iter().collect(),

0 commit comments

Comments
 (0)