alamb commented on code in PR #10330:
URL: https://github.com/apache/datafusion/pull/10330#discussion_r1586649331
##########
datafusion-examples/examples/expr_api.rs:
##########
@@ -92,7 +90,8 @@ fn evaluate_demo() -> Result<()> {
let expr = col("a").lt(lit(5)).or(col("a").eq(lit(8)));
// First, you make a "physical expression" from the logical `Expr`
- let physical_expr = physical_expr(&batch.schema(), expr)?;
+ let df_schema = DFSchema::try_from(batch.schema())?;
Review Comment:
This shows how the new API is used -
`SessionContext::new().create_physical_expr`
##########
datafusion/common/src/dfschema.rs:
##########
@@ -806,6 +820,12 @@ impl From<&DFSchema> for Schema {
}
}
+impl AsRef<Schema> for DFSchema {
Review Comment:
This allows DFSchema to be treated like a &Schema, which is now possible
after https://github.com/apache/datafusion/pull/9595
##########
datafusion-examples/examples/expr_api.rs:
##########
@@ -248,21 +251,6 @@ fn make_ts_field(name: &str) -> Field {
make_field(name, DataType::Timestamp(TimeUnit::Nanosecond, tz))
}
-/// Build a physical expression from a logical one, after applying
simplification and type coercion
-pub fn physical_expr(schema: &Schema, expr: Expr) -> Result<Arc<dyn
PhysicalExpr>> {
Review Comment:
This PR basically ports this code out of the examples and into
`SessionState` and adds documentation and tests
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -510,6 +515,34 @@ impl SessionContext {
}
}
+ /// Create a [`PhysicalExpr`] from an [`Expr`] after applying type
Review Comment:
New public API with example
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -2024,6 +2089,35 @@ impl SessionState {
}
}
+struct SessionSimpifyProvider<'a> {
Review Comment:
This avoids cloning schema / schema refs
##########
datafusion/core/tests/expr_api/mod.rs:
##########
@@ -0,0 +1,181 @@
+// 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.
+
+use arrow::util::pretty::pretty_format_columns;
+use arrow_array::builder::{ListBuilder, StringBuilder};
+use arrow_array::{ArrayRef, RecordBatch, StringArray, StructArray};
+use arrow_schema::{DataType, Field};
+use datafusion::prelude::*;
+use datafusion_common::DFSchema;
+/// Tests of using and evaluating `Expr`s outside the context of a LogicalPlan
+use std::sync::{Arc, OnceLock};
+
+#[test]
+fn test_eq() {
+ // id = '2'
+ evaluate_expr_test(
+ col("id").eq(lit("2")),
+ vec![
+ "+-------+",
+ "| expr |",
+ "+-------+",
+ "| false |",
+ "| true |",
+ "| false |",
+ "+-------+",
+ ],
+ );
+}
+
+#[test]
+fn test_eq_with_coercion() {
+ // id = 2 (need to coerce the 2 to '2' to evaluate)
+ evaluate_expr_test(
+ col("id").eq(lit(2i32)),
+ vec![
+ "+-------+",
+ "| expr |",
+ "+-------+",
+ "| false |",
+ "| true |",
+ "| false |",
+ "+-------+",
+ ],
+ );
+}
+
+#[test]
+fn test_get_field() {
Review Comment:
this tests fail without function rewrites applied
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -1947,6 +1982,36 @@ impl SessionState {
.await
}
+ /// Create a [`PhysicalExpr`] from an [`Expr`] after applying type
+ /// coercion, and function rewrites.
+ ///
+ /// Note: The expression is not [simplified]
+ ///
+ /// # See Also:
+ /// * [`SessionContext::create_physical_expr`] for a higher-level API
+ /// * [`create_physical_expr`] for a lower-level API
+ ///
+ /// [simplified]: datafusion_optimizer::simplify_expressions
+ pub fn create_physical_expr(
+ &self,
+ expr: Expr,
+ df_schema: &DFSchema,
+ ) -> Result<Arc<dyn PhysicalExpr>> {
+ let simplifier =
+ ExprSimplifier::new(SessionSimpifyProvider::new(self, df_schema));
+ // apply type coercion here to ensure types match
+ let mut expr = simplifier.coerce(expr, df_schema)?;
+
+ // rewrite Exprs to functions if necessary
+ let config_options = self.config_options();
Review Comment:
Applying these rewrites is a new feature (and what actually closes
https://github.com/apache/datafusion/issues/10181)
--
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
