Skip to content

Commit

Permalink
fix: address comments
Browse files Browse the repository at this point in the history
Signed-off-by: Zhenchi <[email protected]>
  • Loading branch information
zhongzc committed Dec 29, 2023
1 parent 09639a7 commit 19c5f38
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 11 deletions.
6 changes: 2 additions & 4 deletions src/mito2/src/sst/index/applier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

pub mod builder;

use std::sync::Arc;

use index::inverted_index::search::index_apply::IndexApplier;
use object_store::ObjectStore;

Expand All @@ -30,15 +28,15 @@ pub struct SstIndexApplier {

/// Predefined index applier used to apply predicates to index files
/// and return the relevant row group ids for further scan.
index_applier: Arc<dyn IndexApplier>,
index_applier: Box<dyn IndexApplier>,
}

impl SstIndexApplier {
/// Creates a new [`SstIndexApplier`].
pub fn new(
region_dir: String,
object_store: ObjectStore,
index_applier: Arc<dyn IndexApplier>,
index_applier: Box<dyn IndexApplier>,
) -> Self {
Self {
region_dir,
Expand Down
19 changes: 12 additions & 7 deletions src/mito2/src/sst/index/applier/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ mod between;
// mod regex_match;

use std::collections::HashMap;
use std::sync::Arc;

use api::v1::SemanticType;
use common_query::logical_plan::Expr;
use common_telemetry::warn;
use datafusion_common::ScalarValue;
use datafusion_expr::Expr as DfExpr;
use datatypes::data_type::ConcreteDataType;
Expand Down Expand Up @@ -77,7 +77,7 @@ impl<'a> SstIndexApplierBuilder<'a> {
/// the expressions provided. If no predicates match, returns `None`.
pub fn build(mut self, exprs: &[Expr]) -> Result<Option<SstIndexApplier>> {
for expr in exprs {
self.traverse_and_collect(expr.df_expr())?;
self.traverse_and_collect(expr.df_expr());
}

if self.output.is_empty() {
Expand All @@ -89,23 +89,24 @@ impl<'a> SstIndexApplierBuilder<'a> {
Ok(Some(SstIndexApplier::new(
self.region_dir,
self.object_store,
Arc::new(applier.context(BuildIndexApplierSnafu)?),
Box::new(applier.context(BuildIndexApplierSnafu)?),
)))
}

/// Recursively traverses expressions to collect predicates.
/// Results are stored in `self.output`.
fn traverse_and_collect(&mut self, expr: &DfExpr) -> Result<()> {
match expr {
fn traverse_and_collect(&mut self, expr: &DfExpr) {
let res = match expr {
DfExpr::Between(between) => self.collect_between(between),

// TODO(zhongzc): This PR is too large. The following arms are coming soon.

// DfExpr::InList(in_list) => self.collect_inlist(in_list),
// DfExpr::BinaryExpr(BinaryExpr { left, op, right }) => match op {
// Operator::And => {
// self.traverse_and_collect(left)?;
// self.traverse_and_collect(right)
// self.traverse_and_collect(left);
// self.traverse_and_collect(right);
// Ok(())
// }
// Operator::Or => self.collect_or_eq_list(left, right),
// Operator::Eq => self.collect_eq(left, right),
Expand All @@ -118,6 +119,10 @@ impl<'a> SstIndexApplierBuilder<'a> {

// TODO(zhongzc): support more expressions, e.g. IsNull, IsNotNull, ...
_ => Ok(()),
};

if let Err(err) = res {
warn!("Failed to collect predicates, ignore it. error: {err}, expr: {expr}");
}
}

Expand Down

0 comments on commit 19c5f38

Please sign in to comment.