You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/08/03 03:37:27 UTC

[GitHub] [spark] beliefer opened a new pull request, #37388: [SPARK-39961][SQL] DS V2 push-down translate Cast if the cast is safe

beliefer opened a new pull request, #37388:
URL: https://github.com/apache/spark/pull/37388

   ### What changes were proposed in this pull request?
   Currently, DS V2 push-down translate `Cast` only if the ansi mode is true.
   In fact, if the cast is safe(e.g. cast number to string, cast int to long), we can translate it too.
   
   This PR will call `Cast.canUpCast` so as we can translate `Cast` to V2 `Cast` safely.
   
   Note: The rule `SimplifyCasts` optimize some safe cast, e.g. cast int to long, so we will not see the `Cast` pushed down. 
   
   ### Why are the changes needed?
   Add the range for DS V2 push down `Cast`.
   
   
   ### Does this PR introduce _any_ user-facing change?
   'Yes'.
   `Cast` could be pushed down to data source in more cases.
   
   
   ### How was this patch tested?
   Test cases updated.
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun closed pull request #37388: [SPARK-39961][SQL] DS V2 push-down translate Cast if the cast is safe

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #37388: [SPARK-39961][SQL] DS V2 push-down translate Cast if the cast is safe
URL: https://github.com/apache/spark/pull/37388


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] huaxingao commented on a diff in pull request #37388: [SPARK-39961][SQL] DS V2 push-down translate Cast if the cast is safe

Posted by GitBox <gi...@apache.org>.
huaxingao commented on code in PR #37388:
URL: https://github.com/apache/spark/pull/37388#discussion_r937015727


##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala:
##########
@@ -88,7 +88,8 @@ class V2ExpressionBuilder(e: Expression, isPredicate: Boolean = false) {
       } else {
         None
       }
-    case Cast(child, dataType, _, true) =>
+    case Cast(child, dataType, _, ansiEnabled)
+      if Cast.canUpCast(child.dataType, dataType) || ansiEnabled =>

Review Comment:
   nit: 4-space indentation?



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] beliefer commented on a diff in pull request #37388: [SPARK-39961][SQL] DS V2 push-down translate Cast if the cast is safe

Posted by GitBox <gi...@apache.org>.
beliefer commented on code in PR #37388:
URL: https://github.com/apache/spark/pull/37388#discussion_r937249611


##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala:
##########
@@ -88,7 +88,8 @@ class V2ExpressionBuilder(e: Expression, isPredicate: Boolean = false) {
       } else {
         None
       }
-    case Cast(child, dataType, _, true) =>
+    case Cast(child, dataType, _, ansiEnabled)
+      if Cast.canUpCast(child.dataType, dataType) || ansiEnabled =>

Review Comment:
   Got it.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] beliefer commented on pull request #37388: [SPARK-39961][SQL] DS V2 push-down translate Cast if the cast is safe

Posted by GitBox <gi...@apache.org>.
beliefer commented on PR #37388:
URL: https://github.com/apache/spark/pull/37388#issuecomment-1203550341

   ping @huaxingao cc @cloud-fan 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #37388: [SPARK-39961][SQL] DS V2 push-down translate Cast if the cast is safe

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #37388:
URL: https://github.com/apache/spark/pull/37388#discussion_r937156456


##########
sql/core/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala:
##########
@@ -88,7 +88,8 @@ class V2ExpressionBuilder(e: Expression, isPredicate: Boolean = false) {
       } else {
         None
       }
-    case Cast(child, dataType, _, true) =>
+    case Cast(child, dataType, _, ansiEnabled)
+      if Cast.canUpCast(child.dataType, dataType) || ansiEnabled =>

Review Comment:
   +1 for @huaxingao 's comment. 
   
   In addition, let's switch the predicate order, @beliefer .
   ```scala
   - if Cast.canUpCast(child.dataType, dataType) || ansiEnabled =>
   + if ansiEnabled || Cast.canUpCast(child.dataType, dataType) =>
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] beliefer commented on pull request #37388: [SPARK-39961][SQL] DS V2 push-down translate Cast if the cast is safe

Posted by GitBox <gi...@apache.org>.
beliefer commented on PR #37388:
URL: https://github.com/apache/spark/pull/37388#issuecomment-1205943761

   @dongjoon-hyun @huaxingao @cloud-fan Thank you!


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org