You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "jackwener (via GitHub)" <gi...@apache.org> on 2023/05/05 13:14:18 UTC
[GitHub] [arrow-datafusion] jackwener opened a new issue, #6223: RFC: move TypeConversion Checker into `cast_to()`
jackwener opened a new issue, #6223:
URL: https://github.com/apache/arrow-datafusion/issues/6223
### Describe the bug
Current `coerce_type` mixed with too many functions.
I want to split `check type conversion` from `coerce_type`.
such as code
```rs
type_coercion.rs:566
// Support the `IsTrue` `IsNotTrue` `IsFalse` `IsNotFalse` type coercion.
// The above op will be rewrite to the binary op when creating the physical op.
fn get_casted_expr_for_bool_op(expr: &Expr, schema: &DFSchemaRef) -> Result<Expr> {
let left_type = expr.get_type(schema)?;
let right_type = DataType::Boolean;
let coerced_type = coerce_types(&left_type, &Operator::IsDistinctFrom, &right_type)?;
expr.clone().cast_to(&coerced_type, schema)
}
```
In above code, `coerce_types()` is just for checking TypeConversion, because we already know `coerced_type` is `Boolean`.
------
I think we can check `TypeConversion` in `expr.cast_to()`.
Here is SQLServer Data type conversion Doc.
https://learn.microsoft.com/en-us/sql/t-sql/data-types/data-type-conversion-database-engine?view=sql-server-2017
### To Reproduce
_No response_
### Expected behavior
_No response_
### Additional context
_No response_
--
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-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] jackwener commented on issue #6223: RFC: move TypeConversion Checker into `cast_to()`
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener commented on issue #6223:
URL: https://github.com/apache/arrow-datafusion/issues/6223#issuecomment-1534959070
> Right now I can never quite tell if the inputs to an expr have been coerced or not
Agree with it, because we should `get_type()` just for expr have been coerced
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on issue #6223: RFC: move TypeConversion Checker into `cast_to()`
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #6223:
URL: https://github.com/apache/arrow-datafusion/issues/6223#issuecomment-1534956778
Splitting the "what is the result type of this expression" calculation from "what types should the inputs be coerced to to execute it" is a great idea in my opinion.
I also think it would make the code much easier to reason about if the input arguments were coerced once (during type coercion in the analysis phase) and then never again. Right now I can never quite tell if the inputs to an expr have been coerced or not
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on issue #6223: RFC: move TypeConversion Checker into `cast_to()`
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #6223:
URL: https://github.com/apache/arrow-datafusion/issues/6223#issuecomment-1536231115
I think this issue may be fixed
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] jackwener closed issue #6223: RFC: move TypeConversion Checker into `cast_to()`
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener closed issue #6223: RFC: move TypeConversion Checker into `cast_to()`
URL: https://github.com/apache/arrow-datafusion/issues/6223
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] jackwener commented on issue #6223: RFC: Add TypeConversion Checker
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener commented on issue #6223:
URL: https://github.com/apache/arrow-datafusion/issues/6223#issuecomment-1534391233
cc @mingmwang @alamb @liukun4515 @yjshen
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] jackwener closed issue #6223: RFC: move TypeConversion Checker into `cast_to()`
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener closed issue #6223: RFC: move TypeConversion Checker into `cast_to()`
URL: https://github.com/apache/arrow-datafusion/issues/6223
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org