[GitHub] spark pull request: [SPARK-13844][SQL] Generate better code for fi...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/11684 --- 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-13844][SQL] Generate better code for fi...
Github user davies commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-202500100 LGTM, merging 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-202014919 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-202014920 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54281/ 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-202014884 **[Test build #54281 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54281/consoleFull)** for PR 11684 at commit [`2eda22f`](https://github.com/apache/spark/commit/2eda22f8e78fb531b5d4746d1f40381b5c395b1e). * 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-202002017 **[Test build #54281 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54281/consoleFull)** for PR 11684 at commit [`2eda22f`](https://github.com/apache/spark/commit/2eda22f8e78fb531b5d4746d1f40381b5c395b1e). --- 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-201991658 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54275/ 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-201991655 **[Test build #54275 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54275/consoleFull)** for PR 11684 at commit [`9fd7773`](https://github.com/apache/spark/commit/9fd7773c6cb3856d1d4a2cb893c50361d829b01f). * 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-201991657 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-201991629 **[Test build #54275 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54275/consoleFull)** for PR 11684 at commit [`9fd7773`](https://github.com/apache/spark/commit/9fd7773c6cb3856d1d4a2cb893c50361d829b01f). --- 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-13844][SQL] Generate better code for fi...
Github user kiszk commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-201859532 @davies , I resolved conflicts. Would it be possible to review it again? --- 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-201855094 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54263/ 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-201855093 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-201854748 **[Test build #54263 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54263/consoleFull)** for PR 11684 at commit [`a6fa721`](https://github.com/apache/spark/commit/a6fa7215ea382771959c549af6206bbb4e770579). * 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-201824453 **[Test build #54263 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54263/consoleFull)** for PR 11684 at commit [`a6fa721`](https://github.com/apache/spark/commit/a6fa7215ea382771959c549af6206bbb4e770579). --- 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-198081329 **[Test build #53467 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53467/consoleFull)** for PR 11684 at commit [`6c32ec3`](https://github.com/apache/spark/commit/6c32ec39db308ff59768b3c4c75a1fadadfa867c). --- 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-197491086 **[Test build #53327 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53327/consoleFull)** for PR 11684 at commit [`61c5602`](https://github.com/apache/spark/commit/61c56029c37ae5833a323db1a41d5a2a0d82fb70). * 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-13844][SQL] Generate better code for fi...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/11684#discussion_r56397095
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
---
@@ -237,21 +237,44 @@ case class Divide(left: Expression, right:
Expression) extends BinaryArithmetic
} else {
s"($javaType)(${eval1.value} $symbol ${eval2.value})"
}
-s"""
- ${eval2.code}
- boolean ${ev.isNull} = false;
- $javaType ${ev.value} = ${ctx.defaultValue(javaType)};
- if (${eval2.isNull} || $isZero) {
-${ev.isNull} = true;
+if (nullable) {
--- End diff --
This is always true
---
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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-198115143 **[Test build #53469 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53469/consoleFull)** for PR 11684 at commit [`e04a98c`](https://github.com/apache/spark/commit/e04a98c4595a00fa3b4f15aa167f31eef686a3bf). * 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-198111782 **[Test build #53467 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53467/consoleFull)** for PR 11684 at commit [`6c32ec3`](https://github.com/apache/spark/commit/6c32ec39db308ff59768b3c4c75a1fadadfa867c). * 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-197491610 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/53327/ 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-13844][SQL] Generate better code for fi...
Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/11684#discussion_r56379211 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala --- @@ -199,7 +199,12 @@ case class Divide(left: Expression, right: Expression) extends BinaryArithmetic override def symbol: String = "/" override def decimalMethod: String = "$div" - override def nullable: Boolean = true + override def nullable: Boolean = (left.nullable || +!(!right.nullable && right.isInstanceOf[Literal] && + ((right.asInstanceOf[Literal].value.isInstanceOf[Decimal] && --- End diff -- Since I do not have the benchmark that whose hotspot is related to them, I removed them in ```Divide``` and ```Remainder```. --- 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-13844][SQL] Generate better code for fi...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/11684#discussion_r56397286
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
---
@@ -325,22 +338,72 @@ case class Or(left: Expression, right: Expression)
extends BinaryOperator with P
val eval2 = right.gen(ctx)
// The result should be `true`, if any of them is `true` whenever the
other is null or not.
-s"""
- ${eval1.code}
- boolean ${ev.isNull} = false;
- boolean ${ev.value} = true;
+if (!left.nullable && !right.nullable) {
+ ev.isNull = "false"
+ s"""
+${eval1.code}
+boolean ${ev.value} = true;
- if (!${eval1.isNull} && ${eval1.value}) {
+if (!${eval1.value}) {
+ ${eval2.code}
+ ${ev.value} = ${eval2.value};
+}
+ """
+} else {
+ if (!left.nullable) {
--- End diff --
Could you also remove this branch?
---
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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-198115646 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-198112027 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/53467/ 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-197443339 **[Test build #53327 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53327/consoleFull)** for PR 11684 at commit [`61c5602`](https://github.com/apache/spark/commit/61c56029c37ae5833a323db1a41d5a2a0d82fb70). --- 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-13844][SQL] Generate better code for fi...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/11684#discussion_r56378886
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
---
@@ -325,22 +378,76 @@ case class Or(left: Expression, right: Expression)
extends BinaryOperator with P
val eval2 = right.gen(ctx)
// The result should be `true`, if any of them is `true` whenever the
other is null or not.
-s"""
- ${eval1.code}
- boolean ${ev.isNull} = false;
- boolean ${ev.value} = true;
+if (!left.nullable && !right.nullable) {
+ ev.isNull = "false"
+ s"""
+${eval1.code}
+boolean ${ev.value} = true;
- if (!${eval1.isNull} && ${eval1.value}) {
+if (${eval1.value}) {
--- End diff --
Applied this comment
---
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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-197491599 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-13844][SQL] Generate better code for fi...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/11684#discussion_r56378963
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
---
@@ -274,22 +274,75 @@ case class And(left: Expression, right: Expression)
extends BinaryOperator with
val eval2 = right.gen(ctx)
// The result should be `false`, if any of them is `false` whenever
the other is null or not.
-s"""
- ${eval1.code}
- boolean ${ev.isNull} = false;
- boolean ${ev.value} = false;
+if (!left.nullable && !right.nullable) {
+ ev.isNull = "false"
+ s"""
+${eval1.code}
+boolean ${ev.value} = false;
- if (!${eval1.isNull} && !${eval1.value}) {
+if (${eval1.value}) {
+ ${eval2.code}
+ if (${eval2.value}) {
+${ev.value} = true;
+ }
+}
+ """
+} else {
+ if (!left.nullable) {
--- End diff --
Yes, I removed the second and third 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-198115650 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/53469/ 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-198112026 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-198085859 **[Test build #53469 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53469/consoleFull)** for PR 11684 at commit [`e04a98c`](https://github.com/apache/spark/commit/e04a98c4595a00fa3b4f15aa167f31eef686a3bf). --- 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-13844][SQL] Generate better code for fi...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/11684#discussion_r56378913
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
---
@@ -274,22 +274,75 @@ case class And(left: Expression, right: Expression)
extends BinaryOperator with
val eval2 = right.gen(ctx)
// The result should be `false`, if any of them is `false` whenever
the other is null or not.
-s"""
- ${eval1.code}
- boolean ${ev.isNull} = false;
- boolean ${ev.value} = false;
+if (!left.nullable && !right.nullable) {
+ ev.isNull = "false"
+ s"""
+${eval1.code}
+boolean ${ev.value} = false;
- if (!${eval1.isNull} && !${eval1.value}) {
+if (${eval1.value}) {
+ ${eval2.code}
+ if (${eval2.value}) {
--- End diff --
Applied this comment
---
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-13844][SQL] Generate better code for fi...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/11684#discussion_r56205083
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
---
@@ -325,22 +378,76 @@ case class Or(left: Expression, right: Expression)
extends BinaryOperator with P
val eval2 = right.gen(ctx)
// The result should be `true`, if any of them is `true` whenever the
other is null or not.
-s"""
- ${eval1.code}
- boolean ${ev.isNull} = false;
- boolean ${ev.value} = true;
+if (!left.nullable && !right.nullable) {
+ ev.isNull = "false"
+ s"""
+${eval1.code}
+boolean ${ev.value} = true;
- if (!${eval1.isNull} && ${eval1.value}) {
+if (${eval1.value}) {
--- End diff --
```
if (!${eval1.value}) {
${eval2.code}
${ev.value} = ${eval2.value};
}
```
---
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-13844][SQL] Generate better code for fi...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/11684#discussion_r56204833
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
---
@@ -274,22 +274,75 @@ case class And(left: Expression, right: Expression)
extends BinaryOperator with
val eval2 = right.gen(ctx)
// The result should be `false`, if any of them is `false` whenever
the other is null or not.
-s"""
- ${eval1.code}
- boolean ${ev.isNull} = false;
- boolean ${ev.value} = false;
+if (!left.nullable && !right.nullable) {
+ ev.isNull = "false"
+ s"""
+${eval1.code}
+boolean ${ev.value} = false;
- if (!${eval1.isNull} && !${eval1.value}) {
+if (${eval1.value}) {
+ ${eval2.code}
+ if (${eval2.value}) {
--- End diff --
`${ev.value} = ${eval2.value}`
---
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-13844][SQL] Generate better code for fi...
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/11684#discussion_r56204538
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
---
@@ -274,22 +274,75 @@ case class And(left: Expression, right: Expression)
extends BinaryOperator with
val eval2 = right.gen(ctx)
// The result should be `false`, if any of them is `false` whenever
the other is null or not.
-s"""
- ${eval1.code}
- boolean ${ev.isNull} = false;
- boolean ${ev.value} = false;
+if (!left.nullable && !right.nullable) {
+ ev.isNull = "false"
+ s"""
+${eval1.code}
+boolean ${ev.value} = false;
- if (!${eval1.isNull} && !${eval1.value}) {
+if (${eval1.value}) {
+ ${eval2.code}
+ if (${eval2.value}) {
+${ev.value} = true;
+ }
+}
+ """
+} else {
+ if (!left.nullable) {
--- End diff --
The second and third branch do not look better than the fourth one (also no
performance difference), could you remove that?
---
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-13844][SQL] Generate better code for fi...
Github user davies commented on a diff in the pull request: https://github.com/apache/spark/pull/11684#discussion_r56202965 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala --- @@ -199,7 +199,12 @@ case class Divide(left: Expression, right: Expression) extends BinaryArithmetic override def symbol: String = "/" override def decimalMethod: String = "$div" - override def nullable: Boolean = true + override def nullable: Boolean = (left.nullable || +!(!right.nullable && right.isInstanceOf[Literal] && + ((right.asInstanceOf[Literal].value.isInstanceOf[Decimal] && --- End diff -- This is too complicated, do you think it worth to save the the nullable-checking? If it does not have good speed up (by benchmark), I'd like to not do this. --- 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-13844][SQL] Generate better code for fi...
Github user kiszk commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-196594612 @davies, could you please review it? --- 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-195930295 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-13844][SQL] Generate better code for fi...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-195930296 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/53026/ 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-195929790 **[Test build #53026 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53026/consoleFull)** for PR 11684 at commit [`b005e7b`](https://github.com/apache/spark/commit/b005e7bc0b198329968c1f61622b3463d05054ae). * 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-13844][SQL] Generate better code for fi...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/11684#issuecomment-195915534 **[Test build #53026 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53026/consoleFull)** for PR 11684 at commit [`b005e7b`](https://github.com/apache/spark/commit/b005e7bc0b198329968c1f61622b3463d05054ae). --- 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-13844][SQL] Generate better code for fi...
GitHub user kiszk opened a pull request:
https://github.com/apache/spark/pull/11684
[SPARK-13844][SQL] Generate better code for filters with a non-nullable
column
## What changes were proposed in this pull request?
This PR simplifies generated code with a non-nullable column. This PR
addresses three items:
1. Generate simplified code for and / or
2. Generate better code for divide and remainder with non-zero dividend
3. Pass nullable information into BoundReference at WholeStageCodegen
I have attached the generated code with and without this PR
## How was this patch tested?
Tested by existing test suites in sql/core
Here is a motivating example
(0 to 6).map(i => (i.toString, i.toInt)).toDF("k", "v")
.filter("v % 2 == 0").filter("v <= 4").filter("v > 1").show()
Generated code without this PR
java
/* 032 */ protected void processNext() throws java.io.IOException {
/* 033 */ /*** PRODUCE: Project [_1#0 AS k#3,_2#1 AS v#4] */
/* 034 */
/* 035 */ /*** PRODUCE: Filter ((isnotnull((_2#1 % 2)) && ((_2#1 % 2) =
0)) && ((_2#1 <= 4) && (_2#1 > 1))) */
/* 036 */
/* 037 */ /*** PRODUCE: INPUT */
/* 038 */
/* 039 */ while (!shouldStop() && inputadapter_input.hasNext()) {
/* 040 */ InternalRow inputadapter_row = (InternalRow)
inputadapter_input.next();
/* 041 */ /*** CONSUME: Filter ((isnotnull((_2#1 % 2)) && ((_2#1 % 2)
= 0)) && ((_2#1 <= 4) && (_2#1 > 1))) */
/* 042 */ /* input[1, int] */
/* 043 */ int filter_value1 = inputadapter_row.getInt(1);
/* 044 */
/* 045 */ /* isnotnull((input[1, int] % 2)) */
/* 046 */ /* (input[1, int] % 2) */
/* 047 */ boolean filter_isNull3 = false;
/* 048 */ int filter_value3 = -1;
/* 049 */ if (false || 2 == 0) {
/* 050 */ filter_isNull3 = true;
/* 051 */ } else {
/* 052 */ if (false) {
/* 053 */ filter_isNull3 = true;
/* 054 */ } else {
/* 055 */ filter_value3 = (int)(filter_value1 % 2);
/* 056 */ }
/* 057 */ }
/* 058 */ if (!(!(filter_isNull3))) continue;
/* 059 */
/* 060 */ /* ((input[1, int] % 2) = 0) */
/* 061 */ boolean filter_isNull6 = true;
/* 062 */ boolean filter_value6 = false;
/* 063 */ /* (input[1, int] % 2) */
/* 064 */ boolean filter_isNull7 = false;
/* 065 */ int filter_value7 = -1;
/* 066 */ if (false || 2 == 0) {
/* 067 */ filter_isNull7 = true;
/* 068 */ } else {
/* 069 */ if (false) {
/* 070 */ filter_isNull7 = true;
/* 071 */ } else {
/* 072 */ filter_value7 = (int)(filter_value1 % 2);
/* 073 */ }
/* 074 */ }
/* 075 */ if (!filter_isNull7) {
/* 076 */ filter_isNull6 = false; // resultCode could change
nullability.
/* 077 */ filter_value6 = filter_value7 == 0;
/* 078 */
/* 079 */ }
/* 080 */ if (filter_isNull6 || !filter_value6) continue;
/* 081 */
/* 082 */ /* (input[1, int] <= 4) */
/* 083 */ boolean filter_value11 = false;
/* 084 */ filter_value11 = filter_value1 <= 4;
/* 085 */ if (!filter_value11) continue;
/* 086 */
/* 087 */ /* (input[1, int] > 1) */
/* 088 */ boolean filter_value14 = false;
/* 089 */ filter_value14 = filter_value1 > 1;
/* 090 */ if (!filter_value14) continue;
/* 091 */
/* 092 */ filter_metricValue.add(1);
/* 093 */
/* 094 */ /*** CONSUME: Project [_1#0 AS k#3,_2#1 AS v#4] */
/* 095 */
/* 096 */ /* input[0, string] */
/* 097 */ /* input[0, string] */
/* 098 */ boolean filter_isNull = inputadapter_row.isNullAt(0);
/* 099 */ UTF8String filter_value = filter_isNull ? null :
(inputadapter_row.getUTF8String(0));
/* 100 */ project_holder.reset();
/* 101 */
/* 102 */ project_rowWriter.zeroOutNullBytes();
/* 103 */
/* 104 */ if (filter_isNull) {
/* 105 */ project_rowWriter.setNullAt(0);
/* 106 */ } else {
/* 107 */ project_rowWriter.write(0, filter_value);
/* 108 */ }
/* 109 */
/* 110 */ project_rowWriter.write(1, filter_value1);
/* 111 */ project_result.setTotalSize(project_holder.totalSize());
/* 112 */ append(project_result.copy());
/* 113 */ }
/* 114 */ }
/* 115 */ }
Generated code with this PR
java
/* 032 */ protected void processNext() throws java.io.IOException {
/* 033 */ /*** PRODUCE: Project [_1#0 AS k#3,_2#1 AS v#4] */
/* 034 */
/* 035 */ /*** PRODUCE: Filter (((_2#1 % 2) = 0) && ((_2#1 <= 5) &&
(_2#1 >
