From b1501b84281fa1ee074f26d11de33c40fea6b2b3 Mon Sep 17 00:00:00 2001 From: Sean Smith Date: Thu, 9 Jan 2025 18:03:07 -0600 Subject: [PATCH] remove eval2 --- .../intermediate/planner/plan_scan.rs | 5 +- .../operators/hash_join/condition.rs | 4 +- .../src/execution/operators/project.rs | 5 +- .../src/execution/operators/table_inout.rs | 10 +-- .../src/execution/operators/unnest.rs | 4 +- .../src/expr/physical/case_expr.rs | 70 ------------------- .../src/expr/physical/cast_expr.rs | 7 -- .../src/expr/physical/column_expr.rs | 12 ---- .../src/expr/physical/literal_expr.rs | 5 -- .../src/expr/physical/mod.rs | 20 +++--- .../src/expr/physical/scalar_function_expr.rs | 23 ------ .../src/functions/scalar/mod.rs | 6 +- 12 files changed, 21 insertions(+), 150 deletions(-) diff --git a/crates/rayexec_execution/src/execution/intermediate/planner/plan_scan.rs b/crates/rayexec_execution/src/execution/intermediate/planner/plan_scan.rs index c49db5784..313cc6405 100644 --- a/crates/rayexec_execution/src/execution/intermediate/planner/plan_scan.rs +++ b/crates/rayexec_execution/src/execution/intermediate/planner/plan_scan.rs @@ -95,10 +95,7 @@ impl IntermediatePipelineBuildState<'_> { .context("Failed to plan expressions for values")?; let arrs = exprs .into_iter() - .map(|expr| { - let arr = expr.eval(&dummy_batch)?; - Ok(arr.into_owned()) - }) + .map(|expr| expr.eval(&dummy_batch)) .collect::>>()?; row_arrs.push(arrs); } diff --git a/crates/rayexec_execution/src/execution/operators/hash_join/condition.rs b/crates/rayexec_execution/src/execution/operators/hash_join/condition.rs index ddcfa3da8..5360c5a47 100644 --- a/crates/rayexec_execution/src/execution/operators/hash_join/condition.rs +++ b/crates/rayexec_execution/src/execution/operators/hash_join/condition.rs @@ -80,7 +80,7 @@ impl LeftPrecomputedJoinConditions { pub fn precompute_for_left_batch(&mut self, left: &Batch) -> Result<()> { for condition in &mut self.conditions { let precomputed = condition.left.eval(left)?; - condition.left_precomputed.push(precomputed.into_owned()) + condition.left_precomputed.push(precomputed) } Ok(()) @@ -127,7 +127,7 @@ impl LeftPrecomputedJoinConditions { let result = condition .function .function_impl - .execute2(&[&left_precomputed, right_arr.as_ref()])?; + .execute2(&[&left_precomputed, &right_arr])?; results.push(result); } diff --git a/crates/rayexec_execution/src/execution/operators/project.rs b/crates/rayexec_execution/src/execution/operators/project.rs index 5fdc890cd..d3dd4b5b1 100644 --- a/crates/rayexec_execution/src/execution/operators/project.rs +++ b/crates/rayexec_execution/src/execution/operators/project.rs @@ -25,10 +25,7 @@ impl StatelessOperation for ProjectOperation { let arrs = self .exprs .iter() - .map(|expr| { - let arr = expr.eval(&batch)?; - Ok(arr.into_owned()) - }) + .map(|expr| expr.eval(&batch)) .collect::>>()?; Batch::try_from_arrays(arrs) diff --git a/crates/rayexec_execution/src/execution/operators/table_inout.rs b/crates/rayexec_execution/src/execution/operators/table_inout.rs index 2af1a2a8f..dafaca947 100644 --- a/crates/rayexec_execution/src/execution/operators/table_inout.rs +++ b/crates/rayexec_execution/src/execution/operators/table_inout.rs @@ -92,10 +92,7 @@ impl ExecutableOperator for PhysicalTableInOut { let inputs = self .function_inputs .iter() - .map(|expr| { - let arr = expr.eval(&batch)?; - Ok(arr.into_owned()) - }) + .map(|expr| expr.eval(&batch)) .collect::>>()?; let inputs = Batch::try_from_arrays(inputs)?; @@ -115,10 +112,7 @@ impl ExecutableOperator for PhysicalTableInOut { let additional_outputs = self .projected_outputs .iter() - .map(|expr| { - let arr = expr.eval(&batch)?; - Ok(arr.into_owned()) - }) + .map(|expr| expr.eval(&batch)) .collect::>>()?; state.additional_outputs = additional_outputs; diff --git a/crates/rayexec_execution/src/execution/operators/unnest.rs b/crates/rayexec_execution/src/execution/operators/unnest.rs index 01d658100..66f60f3b9 100644 --- a/crates/rayexec_execution/src/execution/operators/unnest.rs +++ b/crates/rayexec_execution/src/execution/operators/unnest.rs @@ -141,11 +141,11 @@ impl ExecutableOperator for PhysicalUnnest { // Compute inputs. These will be stored until we've processed all rows. for (col_idx, expr) in self.project_expressions.iter().enumerate() { - state.project_inputs[col_idx] = expr.eval(&batch)?.into_owned(); + state.project_inputs[col_idx] = expr.eval(&batch)?; } for (col_idx, expr) in self.unnest_expressions.iter().enumerate() { - state.unnest_inputs[col_idx] = expr.eval(&batch)?.into_owned(); + state.unnest_inputs[col_idx] = expr.eval(&batch)?; } state.input_num_rows = batch.num_rows(); diff --git a/crates/rayexec_execution/src/expr/physical/case_expr.rs b/crates/rayexec_execution/src/expr/physical/case_expr.rs index 8b05cdf61..b2eb80028 100644 --- a/crates/rayexec_execution/src/expr/physical/case_expr.rs +++ b/crates/rayexec_execution/src/expr/physical/case_expr.rs @@ -177,76 +177,6 @@ impl PhysicalCaseExpr { Ok(()) } - - pub fn eval2<'a>(&self, batch: &'a Batch) -> Result> { - let mut arrays = Vec::new(); - let mut indices: Vec<(usize, usize)> = (0..batch.num_rows()).map(|_| (0, 0)).collect(); - - // Track remaining rows we need to evaluate. - // - // True bits are rows we still need to consider. - let mut remaining = Bitmap::new_with_all_true(batch.num_rows()); - - let mut trues_sel = SelectionVector::with_capacity(batch.num_rows()); - - for case in &self.cases { - // Generate selection from remaining bitmap. - let selection = Arc::new(SelectionVector::from_iter(remaining.index_iter())); - - // Get batch with only remaining rows that we should consider. - let selected_batch = batch.select(selection.clone()); - - // Execute 'when'. - let selected = case.when.eval(&selected_batch)?; - - // Determine which rows should be executed for 'then', and which we - // need to fall through on. - SelectExecutor::select(&selected, &mut trues_sel)?; - - // Select rows in batch to execute on based on 'trues'. - let execute_batch = selected_batch.select(Arc::new(trues_sel.clone())); - let output = case.then.eval(&execute_batch)?; - - // Store array for later interleaving. - let array_idx = arrays.len(); - arrays.push(output.into_owned()); - - // Figure out mapping from the 'trues' selection to the original row - // index. - // - // The selection vector locations should index into the full-length - // selection vector to get the original row index. - for (array_row_idx, selected_row_idx) in trues_sel.iter_locations().enumerate() { - // Final output row. - let output_row_idx = selection.get(selected_row_idx); - indices[output_row_idx] = (array_idx, array_row_idx); - - // Update bitmap, this row was handled. - remaining.set_unchecked(output_row_idx, false); - } - } - - // Do all remaining rows. - if remaining.count_trues() != 0 { - let selection = Arc::new(SelectionVector::from_iter(remaining.index_iter())); - let remaining_batch = batch.select(selection.clone()); - - let output = self.else_expr.eval(&remaining_batch)?; - let array_idx = arrays.len(); - arrays.push(output.into_owned()); - - // Update indices. - for (array_row_idx, output_row_idx) in selection.iter_locations().enumerate() { - indices[output_row_idx] = (array_idx, array_row_idx); - } - } - - // Interleave. - let refs: Vec<_> = arrays.iter().collect(); - let arr = interleave(&refs, &indices)?; - - Ok(Cow::Owned(arr)) - } } impl fmt::Display for PhysicalCaseExpr { diff --git a/crates/rayexec_execution/src/expr/physical/cast_expr.rs b/crates/rayexec_execution/src/expr/physical/cast_expr.rs index 135988fc8..8dbc02cec 100644 --- a/crates/rayexec_execution/src/expr/physical/cast_expr.rs +++ b/crates/rayexec_execution/src/expr/physical/cast_expr.rs @@ -69,13 +69,6 @@ impl PhysicalCastExpr { Ok(()) } - - pub fn eval2<'a>(&self, batch: &'a Batch) -> Result> { - let input = self.expr.eval(batch)?; - unimplemented!() - // let out = cast_array(input.as_ref(), self.to.clone(), CastFailBehavior::Error)?; - // Ok(Cow::Owned(out)) - } } impl fmt::Display for PhysicalCastExpr { diff --git a/crates/rayexec_execution/src/expr/physical/column_expr.rs b/crates/rayexec_execution/src/expr/physical/column_expr.rs index 4f2d93036..34024601f 100644 --- a/crates/rayexec_execution/src/expr/physical/column_expr.rs +++ b/crates/rayexec_execution/src/expr/physical/column_expr.rs @@ -44,18 +44,6 @@ impl PhysicalColumnExpr { Ok(()) } - - pub fn eval2<'a>(&self, batch: &'a Batch) -> Result> { - let col = batch.array(self.idx).ok_or_else(|| { - RayexecError::new(format!( - "Tried to get column at index {} in a batch with {} columns", - self.idx, - batch.arrays().len() - )) - })?; - - Ok(Cow::Borrowed(col)) - } } impl fmt::Display for PhysicalColumnExpr { diff --git a/crates/rayexec_execution/src/expr/physical/literal_expr.rs b/crates/rayexec_execution/src/expr/physical/literal_expr.rs index 27d8df54d..1cb352c7a 100644 --- a/crates/rayexec_execution/src/expr/physical/literal_expr.rs +++ b/crates/rayexec_execution/src/expr/physical/literal_expr.rs @@ -46,11 +46,6 @@ impl PhysicalLiteralExpr { Ok(()) } - - pub fn eval2<'a>(&self, batch: &'a Batch) -> Result> { - let arr = self.literal.as_array(batch.num_rows())?; - Ok(Cow::Owned(arr)) - } } impl fmt::Display for PhysicalLiteralExpr { diff --git a/crates/rayexec_execution/src/expr/physical/mod.rs b/crates/rayexec_execution/src/expr/physical/mod.rs index 8984daf54..68f586aad 100644 --- a/crates/rayexec_execution/src/expr/physical/mod.rs +++ b/crates/rayexec_execution/src/expr/physical/mod.rs @@ -13,7 +13,7 @@ use std::fmt; use case_expr::PhysicalCaseExpr; use cast_expr::PhysicalCastExpr; use column_expr::PhysicalColumnExpr; -use evaluator::ExpressionState; +use evaluator::{ExpressionEvaluator, ExpressionState}; use literal_expr::PhysicalLiteralExpr; use rayexec_error::{not_implemented, OptionExt, Result}; use scalar_function_expr::PhysicalScalarFunctionExpr; @@ -57,14 +57,16 @@ impl PhysicalScalarExpression { } } - pub fn eval<'a>(&self, batch: &'a Batch) -> Result> { - match self { - Self::Case(e) => e.eval2(batch), - Self::Cast(e) => e.eval2(batch), - Self::Column(e) => e.eval2(batch), - Self::Literal(e) => e.eval2(batch), - Self::ScalarFunction(e) => e.eval2(batch), - } + // TODO: Remove, needs to happen after operator revamp. + pub fn eval(&self, batch: &Batch) -> Result { + unimplemented!() + // match self { + // Self::Case(e) => e.eval2(batch), + // Self::Cast(e) => e.eval2(batch), + // Self::Column(e) => e.eval2(batch), + // Self::Literal(e) => e.eval2(batch), + // Self::ScalarFunction(e) => e.eval2(batch), + // } } /// Produce a selection vector for the batch using this expression. diff --git a/crates/rayexec_execution/src/expr/physical/scalar_function_expr.rs b/crates/rayexec_execution/src/expr/physical/scalar_function_expr.rs index c7d3ec216..49c40118c 100644 --- a/crates/rayexec_execution/src/expr/physical/scalar_function_expr.rs +++ b/crates/rayexec_execution/src/expr/physical/scalar_function_expr.rs @@ -65,29 +65,6 @@ impl PhysicalScalarFunctionExpr { Ok(()) } - - pub fn eval2<'a>(&self, batch: &'a Batch) -> Result> { - let inputs = self - .inputs - .iter() - .map(|input| input.eval(batch)) - .collect::>>()?; - - let refs: Vec<_> = inputs.iter().map(|a| a.as_ref()).collect(); // Can I not? - let mut out = self.function.function_impl.execute2(&refs)?; - - // If function is provided no input, it's expected to return an - // array of length 1. We extend the array here so that it's the - // same size as the rest. - // - // TODO: Could just extend the selection vector too. - if refs.is_empty() { - let scalar = out.logical_value(0)?; - out = scalar.as_array(batch.num_rows())?; - } - - Ok(Cow::Owned(out)) - } } impl fmt::Display for PhysicalScalarFunctionExpr { diff --git a/crates/rayexec_execution/src/functions/scalar/mod.rs b/crates/rayexec_execution/src/functions/scalar/mod.rs index 3bb20bbb6..03585760b 100644 --- a/crates/rayexec_execution/src/functions/scalar/mod.rs +++ b/crates/rayexec_execution/src/functions/scalar/mod.rs @@ -104,7 +104,7 @@ impl Hash for PlannedScalarFunction { } pub trait ScalarFunctionImpl: Debug + Sync + Send + DynClone { - fn execute2(&self, inputs: &[&Array]) -> Result { + fn execute2(&self, _inputs: &[&Array]) -> Result { unimplemented!() } @@ -118,9 +118,7 @@ pub trait ScalarFunctionImpl: Debug + Sync + Send + DynClone { /// /// The batch's `selection` method should be called to determine which rows /// should be looked at during function eval. - fn execute(&self, input: &Batch, output: &mut Array) -> Result<()> { - unimplemented!() - } + fn execute(&self, input: &Batch, output: &mut Array) -> Result<()>; } impl Clone for Box {