[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #3703: Remove dead code in `UnwrapCastExprRewriter`

[GitBox <gi...@apache.org>]

alamb commented on code in PR #3703:
URL: https://github.com/apache/arrow-datafusion/pull/3703#discussion_r989016120


##########
datafusion/optimizer/src/unwrap_cast_in_comparison.rs:
##########
@@ -122,10 +122,6 @@ impl ExprRewriter for UnwrapCastExprRewriter {
                 let right = right.as_ref().clone();
                 let left_type = left.get_type(&self.schema);
                 let right_type = right.get_type(&self.schema);
-                // can't get the data type, just return the expr
-                if left_type.is_err() || right_type.is_err() {
-                    return Ok(expr.clone());
-                }

Review Comment:
   > Why we need to remove this code? @alamb
   
   My thinking was that any such error is the result of some other issue with the planning or other pass; Thus,  if we ignore the error in the `UnwrapCastExprRewriter` pass we make it harder to find and debug the real underlying problem.
   
   It also has no test coverage (no test fails when this code is removed) which also makes me think all it will do is make debugging future errors harder
   
   @liukun4515  if you think strongly this code should still be here it is fine with me and I will close the PR



-- 
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