Skip to content

Commit

Permalink
rename to eval_v2 and remove todo
Browse files Browse the repository at this point in the history
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
  • Loading branch information
BugenZhao committed Apr 11, 2023
1 parent 446b95a commit 5a25ac9
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/expr/src/expr/expr_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl Expression for LiteralExpression {
self.return_type.clone()
}

async fn eval_new(&self, input: &DataChunk) -> Result<ValueImpl> {
async fn eval_v2(&self, input: &DataChunk) -> Result<ValueImpl> {
Ok(ValueImpl::Scalar {
value: self.literal.clone(),
capacity: input.capacity(),
Expand Down
10 changes: 5 additions & 5 deletions src/expr/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ use super::{ExprError, Result};

/// Interface of an expression.
///
/// There're two functions to evaluate an expression: `eval` and `eval_new`, exactly one of them
/// should be implemented. Prefer calling and implementing `eval_new` instead of `eval` if possible,
/// There're two functions to evaluate an expression: `eval` and `eval_v2`, exactly one of them
/// should be implemented. Prefer calling and implementing `eval_v2` instead of `eval` if possible,
/// to gain the performance benefit of scalar expression.
#[async_trait::async_trait]
pub trait Expression: std::fmt::Debug + Sync + Send {
Expand All @@ -103,9 +103,9 @@ pub trait Expression: std::fmt::Debug + Sync + Send {

/// Evaluate the expression in vectorized execution. Returns an array.
///
/// The default implementation calls `eval_new` and always converts the result to an array.
/// The default implementation calls `eval_v2` and always converts the result to an array.
async fn eval(&self, input: &DataChunk) -> Result<ArrayRef> {
let value = self.eval_new(input).await?;
let value = self.eval_v2(input).await?;
Ok(match value {
ValueImpl::Array(array) => array,
ValueImpl::Scalar { value, capacity } => {
Expand All @@ -120,7 +120,7 @@ pub trait Expression: std::fmt::Debug + Sync + Send {
/// array, or a scalar if all values in the array are the same.
///
/// The default implementation calls `eval` and puts the result into the `Array` variant.
async fn eval_new(&self, input: &DataChunk) -> Result<ValueImpl> {
async fn eval_v2(&self, input: &DataChunk) -> Result<ValueImpl> {
self.eval(input).map_ok(ValueImpl::Array).await
}

Expand Down
7 changes: 3 additions & 4 deletions src/expr/src/expr/template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use std::sync::Arc;

use itertools::{multizip, Itertools};
use paste::paste;
use risingwave_common::array::{Array, ArrayBuilder, ArrayImpl, DataChunk, Utf8Array};
use risingwave_common::array::{Array, ArrayBuilder, DataChunk, Utf8Array};
use risingwave_common::row::OwnedRow;
use risingwave_common::types::{option_as_scalar_ref, DataType, Datum, Scalar};
use risingwave_common::util::iter_util::ZipEqDebug;
Expand All @@ -30,15 +30,15 @@ use crate::expr::{BoxedExpression, Expression, ValueImpl, ValueRef};

macro_rules! gen_eval {
{ ($macro:ident, $macro_row:ident), $ty_name:ident, $OA:ty, $($arg:ident,)* } => {
fn eval_new<'a, 'b, 'async_trait>(&'a self, data_chunk: &'b DataChunk)
fn eval_v2<'a, 'b, 'async_trait>(&'a self, data_chunk: &'b DataChunk)
-> Pin<Box<dyn Future<Output = $crate::Result<ValueImpl>> + Send + 'async_trait>>
where
'a: 'async_trait,
'b: 'async_trait,
{
Box::pin(async move { paste! {
$(
let [<ret_ $arg:lower>] = self.[<expr_ $arg:lower>].eval_new(data_chunk).await?;
let [<ret_ $arg:lower>] = self.[<expr_ $arg:lower>].eval_v2(data_chunk).await?;
let [<val_ $arg:lower>]: ValueRef<'_, $arg> = (&[<ret_ $arg:lower>]).into();
)*

Expand All @@ -58,7 +58,6 @@ macro_rules! gen_eval {
}

// Otherwise, fallback to array computation.
// TODO: match all possible combinations to further get rid of the overhead of `Either` iterators.
($([<val_ $arg:lower>], )*) => {
let bitmap = data_chunk.visibility();
let mut output_array = <$OA as Array>::Builder::with_meta(data_chunk.capacity(), (&self.return_type).into());
Expand Down

0 comments on commit 5a25ac9

Please sign in to comment.