alamb commented on code in PR #3741:
URL: https://github.com/apache/arrow-datafusion/pull/3741#discussion_r990384817
##########
datafusion/optimizer/src/simplify_expressions.rs:
##########
@@ -15,73 +15,25 @@
// specific language governing permissions and limitations
// under the License.
-//! Simplify expressions optimizer rule
+//! Simplify expressions optimizer rule and implementation
-use crate::expr_simplifier::ExprSimplifiable;
+use crate::expr_simplifier::{ExprSimplifier, SimplifyContext};
use crate::{expr_simplifier::SimplifyInfo, OptimizerConfig, OptimizerRule};
use arrow::array::new_null_array;
use arrow::datatypes::{DataType, Field, Schema};
use arrow::error::ArrowError;
use arrow::record_batch::RecordBatch;
-use datafusion_common::{DFSchema, DFSchemaRef, DataFusionError, Result,
ScalarValue};
+use datafusion_common::{DFSchema, DataFusionError, Result, ScalarValue};
use datafusion_expr::{
expr_fn::{and, or},
expr_rewriter::{ExprRewritable, ExprRewriter, RewriteRecursion},
lit,
logical_plan::LogicalPlan,
utils::from_plan,
- BuiltinScalarFunction, ColumnarValue, Expr, ExprSchemable, Operator,
Volatility,
+ BuiltinScalarFunction, ColumnarValue, Expr, Operator, Volatility,
};
use datafusion_physical_expr::{create_physical_expr,
execution_props::ExecutionProps};
-/// Provides simplification information based on schema and properties
-pub(crate) struct SimplifyContext<'a, 'b> {
Review Comment:
This was moved into the public API
##########
datafusion/core/tests/simplification.rs:
##########
@@ -1,108 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied. See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-//! This program demonstrates the DataFusion expression simplification API.
Review Comment:
These were inlined into `expr_api.rs` examples
##########
datafusion/optimizer/src/expr_simplifier.rs:
##########
@@ -37,19 +41,27 @@ pub trait SimplifyInfo {
fn execution_props(&self) -> &ExecutionProps;
}
-/// trait for types that can be simplified
-pub trait ExprSimplifiable: Sized {
Review Comment:
Instead of adding a `simplify` method on to `Expr` via this trait, I propose
to have an `ExprSimplifier` struct that has a simplify method.
I found it made the examples in `expr_api.rs` less awkward to write because
the schema wasn't needed
##########
datafusion/optimizer/src/simplify_expressions.rs:
##########
@@ -950,30 +909,12 @@ macro_rules! assert_contains {
};
}
-/// Apply simplification and constant propagation to ([Expr]).
-///
-/// # Arguments
-///
-/// * `expr` - The logical expression
-/// * `schema` - The DataFusion schema for the expr, used to resolve `Column`
references
-/// to qualified or unqualified fields by name.
-/// * `props` - The Arrow schema for the input, used for determining
expression data types
-/// when performing type coercion.
-pub fn simplify_expr(
Review Comment:
this was added in
https://github.com/apache/arrow-datafusion/commit/fef45e74d6772cf1f4aa8b32338ac4509fa24ab4
by @ygf11 but when I was trying to write examples using it, it ended up being
quite akward
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
