dharanad commented on code in PR #11487:
URL: https://github.com/apache/datafusion/pull/11487#discussion_r1683834548
##########
datafusion/expr/src/planner.rs:
##########
@@ -173,6 +173,24 @@ pub trait ExprPlanner: Send + Sync {
fn plan_overlay(&self, args: Vec<Expr>) ->
Result<PlannerResult<Vec<Expr>>> {
Ok(PlannerResult::Original(args))
}
+
+ fn plan_get_field(&self, args: Vec<Expr>) ->
Result<PlannerResult<Vec<Expr>>> {
+ Ok(PlannerResult::Original(args))
+ }
+ /// Plans compound identifier eg `db.schema.table`.
+ ///
+ /// Note:
+ /// Currently compound identifier for outer query schema is not supported.
+ ///
+ /// Returns empty expression arguments if not possible
Review Comment:
> Since this doesn't have to pass back the arguments, how about simply
returning an Error if it is not possible to plan
It makes sense to me. It's important to make the caller aware that we don't
have a default implementation for compound identifiers with nested names.
> Also, I feel like it would be nice to be able to plan basic SQL like
column references without having to implement plan_compound_indentifier
Its a great idea. I think it simple and reduces complexity.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
