alamb commented on code in PR #11578:
URL: https://github.com/apache/datafusion/pull/11578#discussion_r1687007005
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -518,6 +510,14 @@ impl LogicalPlan {
Ok(using_columns)
}
+ fn as_col(expr: Expr) -> Result<Column> {
Review Comment:
I found this name is somewhat confusing given
[Expr::try_as_col](https://docs.rs/datafusion/latest/datafusion/logical_expr/enum.Expr.html#method.try_as_col)
I suggest:
1. Making this a free function (not in `LogicalPlan`)
2. Add a comment explaining what it does
3. Avoid the copy if possible
For example
```rust
/// Returns the underlying column if `expr` is `Expr::Column` or a cast of
an `Expr::Column`
///
/// Returns an error if expr is not a column or a cast of a column
fn get_as_join_column(expr: &Expr) -> Result<&Column> {
...
}
```
--
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]
