You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by rekhajoshm <gi...@git.apache.org> on 2017/10/03 04:49:15 UTC
[GitHub] spark pull request #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGene...
GitHub user rekhajoshm opened a pull request:
https://github.com/apache/spark/pull/19418
[SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: failed to compile: org.codehaus.commons.compiler.CompileException: for generated java file
## What changes were proposed in this pull request?
From the erratic error observed and quick analysis of code, it seems like leftKeys(i).dataType in SortMergeJoinExec is an AtomicType.It also seems that casting/promotion have played a role, as variable gets reported as long, but does not match the expected case flow of primitives
{code}
case dt: DataType if isPrimitiveType(dt) => s"($c1 > $c2 ? 1 : $c1 < $c2 ? -1 : 0)"
{code}
The fix is to not invoke compare if method does not exist.
## How was this patch tested?
existing test
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rekhajoshm/spark SPARK-19984
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19418.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 #19418
----
commit e3677c9fa9697e0d34f9df52442085a6a481c9e9
Author: Rekha Joshi <re...@gmail.com>
Date: 2015-05-05T23:10:08Z
Merge pull request #1 from apache/master
Pulling functionality from apache spark
commit 106fd8eee8f6a6f7c67cfc64f57c1161f76d8f75
Author: Rekha Joshi <re...@gmail.com>
Date: 2015-05-08T21:49:09Z
Merge pull request #2 from apache/master
pull latest from apache spark
commit 0be142d6becba7c09c6eba0b8ea1efe83d649e8c
Author: Rekha Joshi <re...@gmail.com>
Date: 2015-06-22T00:08:08Z
Merge pull request #3 from apache/master
Pulling functionality from apache spark
commit 6c6ee12fd733e3f9902e10faf92ccb78211245e3
Author: Rekha Joshi <re...@gmail.com>
Date: 2015-09-17T01:03:09Z
Merge pull request #4 from apache/master
Pulling functionality from apache spark
commit b123c601e459d1ad17511fd91dd304032154882a
Author: Rekha Joshi <re...@gmail.com>
Date: 2015-11-25T18:50:32Z
Merge pull request #5 from apache/master
pull request from apache/master
commit c73c32aadd6066e631956923725a48d98a18777e
Author: Rekha Joshi <re...@gmail.com>
Date: 2016-03-18T19:13:51Z
Merge pull request #6 from apache/master
pull latest from apache spark
commit 7dbf7320057978526635bed09dabc8cf8657a28a
Author: Rekha Joshi <re...@gmail.com>
Date: 2016-04-05T20:26:40Z
Merge pull request #8 from apache/master
pull latest from apache spark
commit 5e9d71827f8e2e4d07027281b80e4e073e7fecd1
Author: Rekha Joshi <re...@gmail.com>
Date: 2017-05-01T23:00:30Z
Merge pull request #9 from apache/master
Pull apache spark
commit 63d99b3ce5f222d7126133170a373591f0ac67dd
Author: Rekha Joshi <re...@gmail.com>
Date: 2017-09-30T22:26:44Z
Merge pull request #10 from apache/master
pull latest apache spark
commit 0fa4d6154a4fe9d46c020dc979a0a835776cd83d
Author: rjoshi2 <re...@gmail.com>
Date: 2017-10-03T04:40:53Z
[SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: failed to compile: org.codehaus.commons.compiler.CompileException: for generated java file
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19418
**[Test build #82408 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82408/testReport)** for PR 19418 at commit [`0fa4d61`](https://github.com/apache/spark/commit/0fa4d6154a4fe9d46c020dc979a0a835776cd83d).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19418
**[Test build #82409 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82409/testReport)** for PR 19418 at commit [`c717e9b`](https://github.com/apache/spark/commit/c717e9b8011942536d6b94831c671b4d8fdd7047).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19418
**[Test build #82409 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82409/testReport)** for PR 19418 at commit [`c717e9b`](https://github.com/apache/spark/commit/c717e9b8011942536d6b94831c671b4d8fdd7047).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/19418
From the stacktrace posted in the JIRA, the problematic code is:
/* 151 */ comp = agg_bufValue.compare(smj_value3);
`agg_bufValue` is a `long` but `smj_value3` is a `UTF8String`. It looks strange because we should not to compare two variable in different types at all.
Looks like you don't have re-producible codes for this issue. So I don't think you found the root cause.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19418
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82467/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGene...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm commented on a diff in the pull request:
https://github.com/apache/spark/pull/19418#discussion_r142848020
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -697,7 +697,12 @@ class CodegenContext {
}
"""
s"${addNewFunction(compareFunc, funcCode)}($c1, $c2)"
- case other if other.isInstanceOf[AtomicType] => s"$c1.compare($c2)"
+ case other if other.isInstanceOf[AtomicType] =>
+ if (s"$c1".getClass.getMethods.map(_.getName).filter(_ matches "compare").size == 1) {
--- End diff --
@viirya i was putting a note to that effect :-) This would not work, but trying out something, especially the consistency betn local and jenkins test behavior.I needed "other" to work.Anyhow, mainly looking to check if atomictypes conv/long/String can happen by different attempts.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19418
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82408/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGene...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm closed the pull request at:
https://github.com/apache/spark/pull/19418
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19418
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19418
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82409/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19418
**[Test build #82467 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82467/testReport)** for PR 19418 at commit [`773b429`](https://github.com/apache/spark/commit/773b429991510864878928b2518ff14825e1f865).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19418
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82471/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19418
**[Test build #82471 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82471/testReport)** for PR 19418 at commit [`3f8ce9a`](https://github.com/apache/spark/commit/3f8ce9a99acc1d5ae22004f4f18d94df8966cbcd).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/19418
I think we don't want to add such defensive condition and avoid the logtrace. Without that, we don't know the problem is happened. We should identify the issue and fix it if it is really there, instead of hiding the error. Btw, you still don't get correct result even by avoiding the error, right?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19418
**[Test build #82471 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82471/testReport)** for PR 19418 at commit [`3f8ce9a`](https://github.com/apache/spark/commit/3f8ce9a99acc1d5ae22004f4f18d94df8966cbcd).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19418
**[Test build #82467 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82467/testReport)** for PR 19418 at commit [`773b429`](https://github.com/apache/spark/commit/773b429991510864878928b2518ff14825e1f865).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGene...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/19418#discussion_r142841674
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -697,7 +697,14 @@ class CodegenContext {
}
"""
s"${addNewFunction(compareFunc, funcCode)}($c1, $c2)"
- case other if other.isInstanceOf[AtomicType] => s"$c1.compare($c2)"
+ case other if other.isInstanceOf[AtomicType] =>
+ s"""
+ if ($c1.getClass.getMethods.map(_.getName).filter(_ matches "compare").size == 1) {
+ $c1.compare($c2)
--- End diff --
hmm, `genComp` seems to return a java statement evaluated to boolean, are you sure returning a block like this work?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19418
**[Test build #82408 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82408/testReport)** for PR 19418 at commit [`0fa4d61`](https://github.com/apache/spark/commit/0fa4d6154a4fe9d46c020dc979a0a835776cd83d).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGene...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/19418#discussion_r142844714
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -697,7 +697,14 @@ class CodegenContext {
}
"""
s"${addNewFunction(compareFunc, funcCode)}($c1, $c2)"
- case other if other.isInstanceOf[AtomicType] => s"$c1.compare($c2)"
+ case other if other.isInstanceOf[AtomicType] =>
+ s"""
+ if ($c1.getClass.getMethods.map(_.getName).filter(_ matches "compare").size == 1) {
+ $c1.compare($c2)
--- End diff --
+1
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19418
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGene...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm commented on a diff in the pull request:
https://github.com/apache/spark/pull/19418#discussion_r142475030
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -697,7 +697,12 @@ class CodegenContext {
}
"""
s"${addNewFunction(compareFunc, funcCode)}($c1, $c2)"
- case other if other.isInstanceOf[AtomicType] => s"$c1.compare($c2)"
+ case other if other.isInstanceOf[AtomicType] =>
+ if (other.getClass.getMethods.map(_.getName).filter(_ matches "compare").size == 1) {
--- End diff --
@viirya thats a good point.will rethink.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGene...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/19418#discussion_r142847702
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -697,7 +697,12 @@ class CodegenContext {
}
"""
s"${addNewFunction(compareFunc, funcCode)}($c1, $c2)"
- case other if other.isInstanceOf[AtomicType] => s"$c1.compare($c2)"
+ case other if other.isInstanceOf[AtomicType] =>
+ if (s"$c1".getClass.getMethods.map(_.getName).filter(_ matches "compare").size == 1) {
--- End diff --
You just try to get class from a `String` object.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm commented on the issue:
https://github.com/apache/spark/pull/19418
@viirya You are correct, i am on latest master, and i did not get it yet. This PR was to have a defensive condition. As, if this happens only under certain unique data/flow, this is the one place to certainly avoid the whole stack of logtrace. As I understand, the whole slew of log seems to be the painpoint, nerves the user, and especially since the job nonetheless finishes successfully.thanks
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19418
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGene...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/19418#discussion_r142322714
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala ---
@@ -697,7 +697,12 @@ class CodegenContext {
}
"""
s"${addNewFunction(compareFunc, funcCode)}($c1, $c2)"
- case other if other.isInstanceOf[AtomicType] => s"$c1.compare($c2)"
+ case other if other.isInstanceOf[AtomicType] =>
+ if (other.getClass.getMethods.map(_.getName).filter(_ matches "compare").size == 1) {
--- End diff --
This is going to get a `compare` method from the given data type. It is not the actual runtime object.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19418
Could you please add test suite, too?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19418: [SPARK-19984][SQL] Fix for ERROR codegen.CodeGenerator: ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19418
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org