alamb commented on code in PR #4701:
URL: https://github.com/apache/arrow-datafusion/pull/4701#discussion_r1054832858
##########
datafusion/sql/src/planner.rs:
##########
@@ -970,6 +970,46 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
}
}
+ /// Find all `PlaceHolder` tokens in a logical plan, and try to infer
their type from context
+ fn infer_placeholder_types(&self, expr: &mut Expr, schema: &DFSchema) ->
Result<()> {
+ match expr {
+ Expr::BinaryExpr(BinaryExpr {
+ ref mut left,
+ ref mut right,
+ ..
+ }) => {
+ self.infer_placeholder_types(left, schema)?;
+ self.infer_placeholder_types(right, schema)?;
+ let lt = left.get_type(schema);
+ let rt = right.get_type(schema);
+ match (left, rt) {
Review Comment:
I was thinking in general this logic is only correct for "comparison" binary
operators. It works well for
```
a > $1
```
However, I couldn't come up with an example where it wouldn't work (even
`Operator::Like` would result in the other argument being a string.
##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -128,7 +128,12 @@ impl ExprSchemable for Expr {
Expr::Like { .. } | Expr::ILike { .. } | Expr::SimilarTo { .. } =>
{
Ok(DataType::Boolean)
}
- Expr::Placeholder { data_type, .. } => Ok(data_type.clone()),
+ Expr::Placeholder { data_type, .. } => {
+ let data_type = data_type.clone().ok_or_else(|| {
+ DataFusionError::Internal("Placeholder type not
resolved".to_owned())
Review Comment:
```suggestion
DataFusionError::Internal("Placeholder type could not be
resolved".to_owned())
```
--
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...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
