westonpace commented on code in PR #10330:
URL: https://github.com/apache/datafusion/pull/10330#discussion_r1586867994
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -510,6 +515,34 @@ impl SessionContext {
}
}
+ /// Create a [`PhysicalExpr`] from an [`Expr`] after applying type
+ /// coercion, and function rewrites.
Review Comment:
```suggestion
/// coercion and function rewrites.
```
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -510,6 +515,34 @@ impl SessionContext {
}
}
+ /// Create a [`PhysicalExpr`] from an [`Expr`] after applying type
+ /// coercion, and function rewrites.
+ ///
+ /// # Example
+ /// ```
+ /// # use std::sync::Arc;
+ /// # use arrow::datatypes::{DataType, Field, Schema};
+ /// # use datafusion::prelude::*;
+ /// # use datafusion_common::DFSchema;
+ /// // a = 1 (i64)
+ /// let expr = col("a").eq(lit(1i64));
+ /// // provide type information that `a` is an Int32
+ /// let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
+ /// let df_schema = DFSchema::try_from(schema).unwrap();
+ /// // Create a PhysicalExpr. Note DataFusion automatically coerces
(casts) `1i64` to `1i32`
Review Comment:
Nice, we have some (probably duplicated) type coercion logic on our side we
might be able to replace.
##########
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]
Review Comment:
Why not?
##########
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
Review Comment:
This is somewaht unrelated to this PR but It's news to me that
"SessionContext" is higher level than "SessionState". The only comparison
between the two that I can find is
https://docs.rs/datafusion/latest/datafusion/execution/context/struct.SessionContext.html#sessioncontext-sessionstate-and-taskcontext
which says:
> A
[SessionContext](https://docs.rs/datafusion/latest/datafusion/execution/context/struct.SessionContext.html)
can be created from a
[SessionConfig](https://docs.rs/datafusion/latest/datafusion/prelude/struct.SessionConfig.html)
and stores the state for a particular query session. A single
[SessionContext](https://docs.rs/datafusion/latest/datafusion/execution/context/struct.SessionContext.html)
can run multiple queries.
>
[SessionState](https://docs.rs/datafusion/latest/datafusion/execution/context/struct.SessionState.html)
contains information available during query planning (creating
[LogicalPlan](https://docs.rs/datafusion/latest/datafusion/logical_expr/enum.LogicalPlan.html)s
and
[ExecutionPlan](https://docs.rs/datafusion/latest/datafusion/physical_plan/trait.ExecutionPlan.html)s).
From this it is not obvious that a "SessionContext" contains a
"SessionState". It's also very confusing when I would use "SessionContext" and
when I would use "SessionState" in my user application. Currently, I've been
going off the philosophy of "if an API method needs a SessionState I'd better
create one and if an API method needs a SessionContext I'd better create one."
Some kind of long form design documentation on these two types would be
interesting (or if something already exists that I've missed or is in a blog
post somewhere that would be cool too).
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -2024,6 +2089,35 @@ impl SessionState {
}
}
+struct SessionSimpifyProvider<'a> {
+ state: &'a SessionState,
+ df_schema: &'a DFSchema,
+}
+
+impl<'a> SessionSimpifyProvider<'a> {
Review Comment:
```suggestion
impl<'a> SessionSimplifyProvider<'a> {
```
##########
datafusion/common/src/dfschema.rs:
##########
@@ -806,6 +820,12 @@ impl From<&DFSchema> for Schema {
}
}
+impl AsRef<Schema> for DFSchema {
Review Comment:
:exploding_head:
--
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
