eejbyfeldt commented on code in PR #13249:
URL: https://github.com/apache/datafusion/pull/13249#discussion_r1834447949
##########
datafusion/optimizer/src/utils.rs:
##########
@@ -129,73 +125,57 @@ pub fn log_plan(description: &str, plan: &LogicalPlan) {
/// `c0 > 8` return true;
/// `c0 IS NULL` return false.
pub fn is_restrict_null_predicate<'a>(
+ input_schema: &DFSchema,
predicate: Expr,
- join_cols_of_predicate: impl IntoIterator<Item = &'a Column>,
+ cols_of_predicate: impl IntoIterator<Item = &'a Column>,
) -> Result<bool> {
if matches!(predicate, Expr::Column(_)) {
return Ok(true);
}
- static DUMMY_COL_NAME: &str = "?";
- let schema = Schema::new(vec![Field::new(DUMMY_COL_NAME, DataType::Null,
true)]);
- let input_schema = DFSchema::try_from(schema.clone())?;
- let column = new_null_array(&DataType::Null, 1);
- let input_batch = RecordBatch::try_new(Arc::new(schema.clone()),
vec![column])?;
- let execution_props = ExecutionProps::default();
- let null_column = Column::from_name(DUMMY_COL_NAME);
-
- let join_cols_to_replace = join_cols_of_predicate
+ let replace_columns = cols_of_predicate
.into_iter()
- .map(|column| (column, &null_column))
- .collect::<HashMap<_, _>>();
+ .map(|col| {
+ let data_type = input_schema.data_type(col)?;
+ Ok((col, data_type))
+ })
+ .collect::<Result<HashMap<_, _>>>()?;
+ let replaced_predicate = replace_expr_with_null(predicate,
&replace_columns)?;
- let replaced_predicate = replace_col(predicate, &join_cols_to_replace)?;
- let coerced_predicate = coerce(replaced_predicate, &input_schema)?;
- let phys_expr =
- create_physical_expr(&coerced_predicate, &input_schema,
&execution_props)?;
+ let execution_props = ExecutionProps::default();
+ let info = SimplifyContext::new(&execution_props)
+ .with_schema(Arc::new(input_schema.clone()));
+ let simplifier = ExprSimplifier::new(info).with_canonicalize(false);
+ let expr = simplifier.simplify(replaced_predicate)?;
- let result_type = phys_expr.data_type(&schema)?;
- if !matches!(&result_type, DataType::Boolean) {
- return Ok(false);
+ if matches!(expr.get_type(input_schema)?, DataType::Null) {
+ return Ok(true);
}
- // If result is single `true`, return false;
- // If result is single `NULL` or `false`, return true;
- Ok(match phys_expr.evaluate(&input_batch)? {
Review Comment:
Why did stop using `phys_expr.evaluate`? Did that not allow us to handle
more cases?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
