Skip to content

Commit

Permalink
chore: introduce not_prunable (#1435)
Browse files Browse the repository at this point in the history
The test actually passes on develop. I am not sure if I can trigger the
bad behavior on develop but it seems wrong to use the possibly mutated
refs.
  • Loading branch information
danking authored Nov 21, 2024
1 parent a5b24a1 commit 21f5216
Showing 1 changed file with 25 additions and 32 deletions.
57 changes: 25 additions & 32 deletions vortex-file/src/pruning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ impl PruningPredicate {
}
}

fn not_prunable() -> PruningPredicateStats {
(
Literal::new_expr(Scalar::bool(false, Nullability::NonNullable)),
Relation::new(),
)
}

// Anything that can't be translated has to be represented as
// boolean true expression, i.e. the value might be in that chunk
fn convert_to_pruning_expression(expr: &ExprRef) -> PruningPredicateStats {
Expand Down Expand Up @@ -162,12 +169,7 @@ fn convert_to_pruning_expression(expr: &ExprRef) -> PruningPredicateStats {
if let Some(col) = bexp.lhs().as_any().downcast_ref::<Column>() {
return PruningPredicateRewriter::try_new(col.field().clone(), bexp.op(), bexp.rhs())
.and_then(PruningPredicateRewriter::rewrite)
.unwrap_or_else(|| {
(
Literal::new_expr(Scalar::bool(false, Nullability::NonNullable)),
Relation::new(),
)
});
.unwrap_or_else(not_prunable);
};

if let Some(col) = bexp.rhs().as_any().downcast_ref::<Column>() {
Expand All @@ -177,38 +179,29 @@ fn convert_to_pruning_expression(expr: &ExprRef) -> PruningPredicateStats {
bexp.lhs(),
)
.and_then(PruningPredicateRewriter::rewrite)
.unwrap_or_else(|| {
(
Literal::new_expr(Scalar::bool(false, Nullability::NonNullable)),
Relation::new(),
)
});
};
.unwrap_or_else(not_prunable);
}
}

(
Literal::new_expr(Scalar::bool(false, Nullability::NonNullable)),
Relation::new(),
)
not_prunable()
}

fn convert_column_reference(expr: &ExprRef, invert: bool) -> PruningPredicateStats {
let mut refs = Relation::new();
let min_expr = replace_column_with_stat(expr, Stat::Min, &mut refs);
let max_expr = replace_column_with_stat(expr, Stat::Max, &mut refs);
(
min_expr
.zip(max_expr)
.map(|(min_exp, max_exp)| {
if invert {
BinaryExpr::new_expr(min_exp, Operator::And, max_exp)
} else {
Not::new_expr(BinaryExpr::new_expr(min_exp, Operator::Or, max_exp))
}
})
.unwrap_or_else(|| Literal::new_expr(Scalar::bool(false, Nullability::NonNullable))),
refs,
)
let Some(min_expr) = replace_column_with_stat(expr, Stat::Min, &mut refs) else {
return not_prunable();
};
let Some(max_expr) = replace_column_with_stat(expr, Stat::Max, &mut refs) else {
return not_prunable();
};

let expr = if invert {
BinaryExpr::new_expr(min_expr, Operator::And, max_expr)
} else {
Not::new_expr(BinaryExpr::new_expr(min_expr, Operator::Or, max_expr))
};

(expr, refs)
}

struct PruningPredicateRewriter<'a> {
Expand Down

0 comments on commit 21f5216

Please sign in to comment.