You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "mingmwang (via GitHub)" <gi...@apache.org> on 2023/04/24 07:32:06 UTC

[GitHub] [arrow-datafusion] mingmwang commented on a diff in pull request #6102: enhance: avoid cast expr in Between as much as possible.

mingmwang commented on code in PR #6102:
URL: https://github.com/apache/arrow-datafusion/pull/6102#discussion_r1174890770


##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -312,49 +313,39 @@ impl TreeNodeRewriter for TypeCoercionRewriter {
             }) => {
                 let expr_type = expr.get_type(&self.schema)?;
                 let low_type = low.get_type(&self.schema)?;
-                let low_coerced_type = comparison_coercion(&expr_type, &low_type)
-                    .ok_or_else(|| {
-                        DataFusionError::Internal(format!(
-                            "Failed to coerce types {expr_type} and {low_type} in BETWEEN expression"
-                        ))
-                    })?;
                 let high_type = high.get_type(&self.schema)?;
-                let high_coerced_type = comparison_coercion(&expr_type, &low_type)
-                    .ok_or_else(|| {
-                        DataFusionError::Internal(format!(
-                            "Failed to coerce types {expr_type} and {high_type} in BETWEEN expression"
-                        ))
-                    })?;
-                let coercion_type =
-                    comparison_coercion(&low_coerced_type, &high_coerced_type)
-                        .ok_or_else(|| {
-                            DataFusionError::Internal(format!(
-                                "Failed to coerce types {expr_type} and {high_type} in BETWEEN expression"
-                            ))
-                        })?;
-                let expr = Expr::Between(Between::new(
-                    Box::new(expr.cast_to(&coercion_type, &self.schema)?),
-                    negated,
-                    Box::new(low.cast_to(&coercion_type, &self.schema)?),
-                    Box::new(high.cast_to(&coercion_type, &self.schema)?),
-                ));
-                Ok(expr)
+                let between_common_type = comparison_coercion(&low_type, &high_type).ok_or(DataFusionError::Internal(format!("Failed to coerce types {low_type} and {high_type} in BETWEEN expression")))?;
+                // Because low/high is Literal, cast them as much as possible to avoid cast expr.

Review Comment:
   Why the high and low are always `Literal`?  



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