[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-114774250 [Test build #955 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/955/console) for PR 6825 at commit [`43170cc`](https://github.com/apache/spark/commit/43170cc61facb26b194d5d2447b9c487a98adf82). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/6825 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-114741059 [Test build #955 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/955/consoleFull) for PR 6825 at commit [`43170cc`](https://github.com/apache/spark/commit/43170cc61facb26b194d5d2447b9c487a98adf82). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user davies commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-114741423 LGTM, the failed PySpark test is not related, merging this into master, thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r33119609
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
---
@@ -175,8 +175,10 @@ class CodeGenContext {
* Generate code for compare expression in Java
*/
def genComp(dataType: DataType, c1: String, c2: String): String =
dataType match {
+// java boolean doesn't support > or < operator
+case BooleanType => s"($c1 == $c2 ? 0 : ($c1 ? 1 : -1))"
--- End diff --
Good catch!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-114740100 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-114740080 [Test build #35636 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/35636/console) for PR 6825 at commit [`43170cc`](https://github.com/apache/spark/commit/43170cc61facb26b194d5d2447b9c487a98adf82). * This patch **fails PySpark unit tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-114726275 [Test build #35636 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/35636/consoleFull) for PR 6825 at commit [`43170cc`](https://github.com/apache/spark/commit/43170cc61facb26b194d5d2447b9c487a98adf82). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-114726230 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-114726224 Merged build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user davies commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-114612866 @cloud-fan Now #6876 is merged, could you update this PR? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-113052909 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-113049606 Merged build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-113049620 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32661616
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
---
@@ -109,7 +109,7 @@ trait ExpressionEvalHelper {
}
val actual = plan(inputRow)
-val expectedRow = new
GenericRow(Array[Any](CatalystTypeConverters.convertToCatalyst(expected)))
+val expectedRow =
InternalRow.fromSeq(Array(CatalystTypeConverters.convertToCatalyst(expected)))
if (actual.hashCode() != expectedRow.hashCode()) {
--- End diff --
If different types of rows have different hash code implementations they we
cannot use them as keys in a hash table. This is to check that they all share
an implementation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32656460
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
---
@@ -109,7 +109,7 @@ trait ExpressionEvalHelper {
}
val actual = plan(inputRow)
-val expectedRow = new
GenericRow(Array[Any](CatalystTypeConverters.convertToCatalyst(expected)))
+val expectedRow =
InternalRow.fromSeq(Array(CatalystTypeConverters.convertToCatalyst(expected)))
--- End diff --
Sound reasonable, it's anonying to have so many `UTF8String.fromString` in
test cases.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32656257
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
---
@@ -109,7 +109,7 @@ trait ExpressionEvalHelper {
}
val actual = plan(inputRow)
-val expectedRow = new
GenericRow(Array[Any](CatalystTypeConverters.convertToCatalyst(expected)))
+val expectedRow =
InternalRow.fromSeq(Array(CatalystTypeConverters.convertToCatalyst(expected)))
if (actual.hashCode() != expectedRow.hashCode()) {
--- End diff --
cc @marmbrus
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32656297
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
---
@@ -55,7 +55,7 @@ trait ExpressionEvalHelper {
val actual = try evaluate(expression, inputRow) catch {
case e: Exception => fail(s"Exception evaluating $expression", e)
}
-if (actual != expected) {
+if (actual !== expected) {
--- End diff --
Good catch!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112887742 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112887650 [Test build #35045 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/35045/console) for PR 6825 at commit [`8c3fbde`](https://github.com/apache/spark/commit/8c3fbdeecc44508b66e95d34da5050890da54915). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32647979
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
---
@@ -109,7 +109,7 @@ trait ExpressionEvalHelper {
}
val actual = plan(inputRow)
-val expectedRow = new
GenericRow(Array[Any](CatalystTypeConverters.convertToCatalyst(expected)))
+val expectedRow =
InternalRow.fromSeq(Array(CatalystTypeConverters.convertToCatalyst(expected)))
if (actual.hashCode() != expectedRow.hashCode()) {
--- End diff --
Does anyone know why we are expecting same hash code for different
`hashCode` implementation?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112857482 [Test build #35045 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/35045/consoleFull) for PR 6825 at commit [`8c3fbde`](https://github.com/apache/spark/commit/8c3fbdeecc44508b66e95d34da5050890da54915). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112856791 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32641466
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
---
@@ -109,7 +109,7 @@ trait ExpressionEvalHelper {
}
val actual = plan(inputRow)
-val expectedRow = new
GenericRow(Array[Any](CatalystTypeConverters.convertToCatalyst(expected)))
+val expectedRow =
InternalRow.fromSeq(Array(CatalystTypeConverters.convertToCatalyst(expected)))
--- End diff --
@davies , we need the conversion here, as we use `String` instead of
`UTF8String` at many tests...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112856764 Merged build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112839240 [Test build #35044 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/35044/console) for PR 6825 at commit [`97066a6`](https://github.com/apache/spark/commit/97066a601123233cdbe24a8dd7c45034326c6b42). * This patch **fails Spark unit tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112839277 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32636153
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
---
@@ -55,7 +55,7 @@ trait ExpressionEvalHelper {
val actual = try evaluate(expression, inputRow) catch {
case e: Exception => fail(s"Exception evaluating $expression", e)
}
-if (actual != expected) {
+if (actual !== expected) {
--- End diff --
We should use `===` and `!==` in tests, which is more powerful and can
handle `Array` equals for us.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32636034
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
---
@@ -123,24 +124,40 @@ class ArithmeticExpressionSuite extends SparkFunSuite
with ExpressionEvalHelper
}
}
- test("MaxOf") {
-checkEvaluation(MaxOf(1, 2), 2)
-checkEvaluation(MaxOf(2, 1), 2)
-checkEvaluation(MaxOf(1L, 2L), 2L)
-checkEvaluation(MaxOf(2L, 1L), 2L)
+ test("MaxOf basic") {
+testNumericDataTypes { convert =>
+ val small = Literal(convert(1))
+ val large = Literal(convert(2))
+ checkEvaluation(MaxOf(small, large), convert(2))
+ checkEvaluation(MaxOf(large, small), convert(2))
+ checkEvaluation(MaxOf(Literal.create(null, small.dataType), large),
convert(2))
+ checkEvaluation(MaxOf(large, Literal.create(null, small.dataType)),
convert(2))
+}
+ }
-checkEvaluation(MaxOf(Literal.create(null, IntegerType), 2), 2)
-checkEvaluation(MaxOf(2, Literal.create(null, IntegerType)), 2)
+ test("MaxOf for atomic type") {
+checkEvaluation(MaxOf(true, false), true)
+checkEvaluation(MaxOf("abc", "bcd"), UTF8String.fromString("bcd"))
+//checkEvaluation(MaxOf(Array(1.toByte, 2.toByte), Array(1.toByte,
3.toByte)),
+// Array(1.toByte, 3.toByte))
--- End diff --
This test will fail at `checkEvaluationWithGeneratedProjection` for hash
code not equal. I'm not sure why we want to compare the hash code, and I looked
into the hashCode implementation of `GenericRow` and `Row`, they are different.
Does anyone know why we want to compare the hash code there?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112833916 [Test build #35044 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/35044/consoleFull) for PR 6825 at commit [`97066a6`](https://github.com/apache/spark/commit/97066a601123233cdbe24a8dd7c45034326c6b42). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112832574 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112832531 Merged build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32595486
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
---
@@ -55,7 +55,7 @@ trait ExpressionEvalHelper {
val actual = try evaluate(expression, inputRow) catch {
case e: Exception => fail(s"Exception evaluating $expression", e)
}
-if (actual != expected) {
+if (actual !== CatalystTypeConverters.convertToCatalyst(expected)) {
--- End diff --
That should be updated. You can update it here, or cleanup all of them
later (another PR).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32595311
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
---
@@ -55,7 +55,7 @@ trait ExpressionEvalHelper {
val actual = try evaluate(expression, inputRow) catch {
case e: Exception => fail(s"Exception evaluating $expression", e)
}
-if (actual != expected) {
+if (actual !== CatalystTypeConverters.convertToCatalyst(expected)) {
--- End diff --
Ok, that makes sense, I will remove tests for Date and Timestamp, and
revert the change here.
btw, why we do the convert
[here](https://github.com/apache/spark/blob/master/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala#L112)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32593557
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
---
@@ -55,7 +55,7 @@ trait ExpressionEvalHelper {
val actual = try evaluate(expression, inputRow) catch {
case e: Exception => fail(s"Exception evaluating $expression", e)
}
-if (actual != expected) {
+if (actual !== CatalystTypeConverters.convertToCatalyst(expected)) {
--- End diff --
We used to use internal type for expressions tests, so
`CatalystTypeConverters.convertToCatalyst` is not needed.
If you really think it's more convenient to use public types, please also
change others test cases, to make them consistent.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32593520
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
---
@@ -123,24 +123,54 @@ class ArithmeticExpressionSuite extends SparkFunSuite
with ExpressionEvalHelper
}
}
- test("MaxOf") {
-checkEvaluation(MaxOf(1, 2), 2)
-checkEvaluation(MaxOf(2, 1), 2)
-checkEvaluation(MaxOf(1L, 2L), 2L)
-checkEvaluation(MaxOf(2L, 1L), 2L)
-
-checkEvaluation(MaxOf(Literal.create(null, IntegerType), 2), 2)
-checkEvaluation(MaxOf(2, Literal.create(null, IntegerType)), 2)
+ test("MaxOf basic") {
+testNumericDataTypes { convert =>
+ val small = Literal(convert(1))
+ val large = Literal(convert(2))
+ checkEvaluation(MaxOf(small, large), convert(2))
+ checkEvaluation(MaxOf(large, small), convert(2))
+ checkEvaluation(MaxOf(Literal.create(null, small.dataType), large),
convert(2))
+ checkEvaluation(MaxOf(large, Literal.create(null, small.dataType)),
convert(2))
+}
+ }
+
+ test("MaxOf for atomic type") {
+checkEvaluation(MaxOf(true, false), true)
+checkEvaluation(MaxOf("abc", "bcd"), "bcd")
+//checkEvaluation(MaxOf(Array(1.toByte, 2.toByte), Array(1.toByte,
3.toByte)),
+// Array(1.toByte, 3.toByte))
+checkEvaluation(MaxOf(java.sql.Date.valueOf("2012-11-12"),
java.sql.Date.valueOf("2012-12-22")),
--- End diff --
All expressions only work with InternalRow, which only contain internal
types (int, long, UTF8String), no Date or Timestamp. so we don't need to test
Date/Timestamp, or use DateUtils.fromDate() to do the conversion before calling
expression on them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112484946 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112484812 [Test build #34987 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34987/console) for PR 6825 at commit [`f153380`](https://github.com/apache/spark/commit/f153380c5e6d0e09622ec5fb4ab8a315d48c3a78). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `case class IsNull(child: Expression) extends UnaryExpression with Predicate ` * `case class IsNotNull(child: Expression) extends UnaryExpression with Predicate ` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112452130 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112452197 [Test build #34987 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34987/consoleFull) for PR 6825 at commit [`f153380`](https://github.com/apache/spark/commit/f153380c5e6d0e09622ec5fb4ab8a315d48c3a78). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112452089 Merged build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user davies commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112266153 @cloud-fan There are some changes related to Max/Min in #6726 , is it useful for you? It will be good if you could merge them here. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112081358 retest it please. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32422035
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
---
@@ -341,9 +341,20 @@ case class MaxOf(left: Expression, right: Expression)
extends BinaryArithmetic {
}
override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode):
String = {
-if (ctx.isNativeType(left.dataType)) {
+if (ctx.isNativeType(dataType)) {
val eval1 = left.gen(ctx)
val eval2 = right.gen(ctx)
+ val result = if (dataType == BooleanType) {
+s"${ev.primitive} = ${eval1.primitive} || ${eval2.primitive};"
--- End diff --
the code `result` is inserted after null check.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32410987
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
---
@@ -356,15 +367,11 @@ case class MaxOf(left: Expression, right: Expression)
extends BinaryArithmetic {
${ev.isNull} = ${eval1.isNull};
${ev.primitive} = ${eval1.primitive};
} else {
- if (${eval1.primitive} > ${eval2.primitive}) {
-${ev.primitive} = ${eval1.primitive};
- } else {
-${ev.primitive} = ${eval2.primitive};
- }
+ $result
}
"""
} else {
- super.genCode(ctx, ev)
+ ctx.defaultCodeGen(ev, this)
--- End diff --
yes,i think you are right. because their symbol is empty.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user lianhuiwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32410564
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
---
@@ -341,9 +341,20 @@ case class MaxOf(left: Expression, right: Expression)
extends BinaryArithmetic {
}
override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode):
String = {
-if (ctx.isNativeType(left.dataType)) {
+if (ctx.isNativeType(dataType)) {
val eval1 = left.gen(ctx)
val eval2 = right.gen(ctx)
+ val result = if (dataType == BooleanType) {
+s"${ev.primitive} = ${eval1.primitive} || ${eval2.primitive};"
--- End diff --
i think at first it needs to check whether eval1 or eval2 isNull. and then
calculate primitive.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112016874 [Test build #34933 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34933/console) for PR 6825 at commit [`03021bc`](https://github.com/apache/spark/commit/03021bc86933f70342a0500c687972e6d7749ea5). * This patch **fails Spark unit tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-112016896 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-111998049 [Test build #34933 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34933/consoleFull) for PR 6825 at commit [`03021bc`](https://github.com/apache/spark/commit/03021bc86933f70342a0500c687972e6d7749ea5). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-111997405 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-111997343 Merged build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-111990866 [Test build #34923 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34923/console) for PR 6825 at commit [`cd51a7d`](https://github.com/apache/spark/commit/cd51a7d69a1100f83c22bb782041037297f09b1a). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds no public classes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-111990868 Merged build finished. Test FAILed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-111990308 [Test build #34923 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34923/consoleFull) for PR 6825 at commit [`cd51a7d`](https://github.com/apache/spark/commit/cd51a7d69a1100f83c22bb782041037297f09b1a). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-111989386 Merged build started. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32399563
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
---
@@ -356,15 +367,11 @@ case class MaxOf(left: Expression, right: Expression)
extends BinaryArithmetic {
${ev.isNull} = ${eval1.isNull};
${ev.primitive} = ${eval1.primitive};
} else {
- if (${eval1.primitive} > ${eval2.primitive}) {
-${ev.primitive} = ${eval1.primitive};
- } else {
-${ev.primitive} = ${eval2.primitive};
- }
+ $result
}
"""
} else {
- super.genCode(ctx, ev)
+ ctx.defaultCodeGen(ev, this)
--- End diff --
The origin code is not right as `super` here refers to `BinaryArithmetic`,
not `Expression` as we wanted.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-111975457 cc @rxin @davies --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/6825#discussion_r32399462
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
---
@@ -123,24 +123,50 @@ class ArithmeticExpressionSuite extends SparkFunSuite
with ExpressionEvalHelper
}
}
- test("MaxOf") {
-checkEvaluation(MaxOf(1, 2), 2)
-checkEvaluation(MaxOf(2, 1), 2)
-checkEvaluation(MaxOf(1L, 2L), 2L)
-checkEvaluation(MaxOf(2L, 1L), 2L)
-
-checkEvaluation(MaxOf(Literal.create(null, IntegerType), 2), 2)
-checkEvaluation(MaxOf(2, Literal.create(null, IntegerType)), 2)
+ test("MaxOf basic") {
+testNumericDataTypes { convert =>
+ val small = Literal(convert(1))
+ val large = Literal(convert(2))
+ checkEvaluation(MaxOf(small, large), convert(2))
+ checkEvaluation(MaxOf(large, small), convert(2))
+ checkEvaluation(MaxOf(Literal.create(null, small.dataType), large),
convert(2))
+ checkEvaluation(MaxOf(large, Literal.create(null, small.dataType)),
convert(2))
+}
+ }
+
+ test("MaxOf for atomic type") {
+checkEvaluation(MaxOf(true, false), true)
+checkEvaluation(MaxOf("abc", "bcd"), "bcd")
+//checkEvaluation(MaxOf(Array(1.toByte, 2.toByte), Array(1.toByte,
3.toByte)),
+// Array(1.toByte, 3.toByte))
--- End diff --
This test will fail at `checkEvaluationWithGeneratedProjection` for hash
code not equal. I'm not sure why we want to compare the hash code, and I looked
into the `hashCode` implementation of `GenericRow` and `Row`, they are
different. I'm trying to figure it out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6825#issuecomment-111974399 Merged build triggered. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8371][SQL] improve unit test for MaxOf ...
GitHub user cloud-fan opened a pull request: https://github.com/apache/spark/pull/6825 [SPARK-8371][SQL] improve unit test for MaxOf and MinOf and fix bugs a follow up of https://github.com/apache/spark/pull/6813 You can merge this pull request into a Git repository by running: $ git pull https://github.com/cloud-fan/spark cg Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/6825.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #6825 commit cd51a7d69a1100f83c22bb782041037297f09b1a Author: Wenchen Fan Date: 2015-06-15T08:03:48Z fix bugs in code gen --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
