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/02/24 19:16:39 UTC

[GitHub] [spark] anchovYu opened a new pull request #35654: [SPARK-38321][SQL][Tests] Fix BooleanSimplificationSuite under ANSI

anchovYu opened a new pull request #35654:
URL: https://github.com/apache/spark/pull/35654


   ### What changes were proposed in this pull request?
   This PR fixes BooleanSimplificationSuite under ANSI mode. Change the test case to be a long type to avoid casting difference in ANSI on/off.
   
   ### Why are the changes needed?
   To set up a new GA job to run tests with ANSI mode before 3.3.0 release.
   
   ### Does this PR introduce _any_ user-facing change?
   No.
   
   ### How was this patch tested?
   Test locally with both ANSI on and off, both passed.


-- 
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] HyukjinKwon commented on a change in pull request #35654: [SPARK-38321][SQL][Tests] Fix BooleanSimplificationSuite under ANSI

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #35654:
URL: https://github.com/apache/spark/pull/35654#discussion_r814456019



##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/BooleanSimplificationSuite.scala
##########
@@ -138,8 +138,8 @@ class BooleanSimplificationSuite extends PlanTest with ExpressionEvalHelper with
       'a > 1 && 'b > 3 && 'c > 1)
 
     checkCondition(
-      ('a > 1 || 'b > 3) && (('a > 1 || 'b > 3) && 'd > 0 && (('a > 1 || 'b > 3) && 'c > 1)),
-      ('a > 1 || 'b > 3) && 'd > 0 && 'c > 1)
+      ('a > 1 || 'b > 3) && (('a > 1 || 'b > 3) && 'd > 0L && (('a > 1 || 'b > 3) && 'c > 1)),
+      ('a > 1 || 'b > 3) && 'd > 0L && 'c > 1)

Review comment:
       Hm. how did this trigger casting? Seems like all `IntegerType`.




-- 
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] gengliangwang closed pull request #35654: [SPARK-38321][SQL][TESTS] Fix BooleanSimplificationSuite under ANSI

Posted by GitBox <gi...@apache.org>.
gengliangwang closed pull request #35654:
URL: https://github.com/apache/spark/pull/35654


   


-- 
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] gengliangwang commented on pull request #35654: [SPARK-38321][SQL][TESTS] Fix BooleanSimplificationSuite under ANSI

Posted by GitBox <gi...@apache.org>.
gengliangwang commented on pull request #35654:
URL: https://github.com/apache/spark/pull/35654#issuecomment-1053926577


   Merging to master


-- 
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] anchovYu commented on a change in pull request #35654: [SPARK-38321][SQL][TESTS] Fix BooleanSimplificationSuite under ANSI

Posted by GitBox <gi...@apache.org>.
anchovYu commented on a change in pull request #35654:
URL: https://github.com/apache/spark/pull/35654#discussion_r815043556



##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/BooleanSimplificationSuite.scala
##########
@@ -138,8 +138,8 @@ class BooleanSimplificationSuite extends PlanTest with ExpressionEvalHelper with
       'a > 1 && 'b > 3 && 'c > 1)
 
     checkCondition(
-      ('a > 1 || 'b > 3) && (('a > 1 || 'b > 3) && 'd > 0 && (('a > 1 || 'b > 3) && 'c > 1)),
-      ('a > 1 || 'b > 3) && 'd > 0 && 'c > 1)
+      ('a > 1 || 'b > 3) && (('a > 1 || 'b > 3) && 'd > 0L && (('a > 1 || 'b > 3) && 'c > 1)),
+      ('a > 1 || 'b > 3) && 'd > 0L && 'c > 1)

Review comment:
       ANSI mode would cast the d(string) and 0 to long type. @gengliangwang for more context.




-- 
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] AmplabJenkins commented on pull request #35654: [SPARK-38321][SQL][TESTS] Fix BooleanSimplificationSuite under ANSI

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35654:
URL: https://github.com/apache/spark/pull/35654#issuecomment-1051238131


   Can one of the admins verify this patch?


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