You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/12/01 09:34:30 UTC
[GitHub] [spark] leanken opened a new pull request #30560: [SPARK-33619] GetMapValueUtil code generation error.
leanken opened a new pull request #30560:
URL: https://github.com/apache/spark/pull/30560
### What changes were proposed in this pull request?
Code Gen bug fix that introduced by SPARK-33460
```
GetMapValueUtil
s"""throw new NoSuchElementException("Key " + $eval2 + " does not exist.");"""
SHOULD BE
s"""throw new java.util.NoSuchElementException("Key " + $eval2 + " does not exist.");"""
```
### Why are the changes needed?
Bug Fix.
### Does this PR introduce any user-facing change?
No.
### How was this patch tested?
Existing UT.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-736983684
**[Test build #132019 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132019/testReport)** for PR 30560 at commit [`8e4ca3d`](https://github.com/apache/spark/commit/8e4ca3d4f9513506c57d0aecd2d46ca8878c1077).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737167724
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737014904
```
org.apache.spark.sql.catalyst.expressions.DateExpressionsSuite.Consistent error handling for datetime formatting and parsing functions 16 ms 1
org.apache.spark.sql.catalyst.expressions.ObjectExpressionsSuite.SPARK-23594 GetExternalRowField should support interpreted execution 49 ms 1
org.apache.spark.sql.catalyst.expressions.ObjectExpressionsSuite.SPARK-23595 ValidateExternalType should support interpreted execution 0.64 sec 1
org.apache.spark.sql.catalyst.expressions.RegexpExpressionsSuite.RegexExtract 0.25 sec 1
org.apache.spark.sql.catalyst.expressions.RegexpExpressionsSuite.RegexExtractAll
```
It seems the five errors? If so, IMO its okay to fix all of them at once. Anyway, nice catch!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737024489
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737042348
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-755042057
Thank you @dongjoon-hyun and @LuciferYang for investigating and checking this!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #30560:
URL: https://github.com/apache/spark/pull/30560
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737289348
**[Test build #132040 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132040/testReport)** for PR 30560 at commit [`95bdd37`](https://github.com/apache/spark/commit/95bdd379d7addcac25de3e52dd27710f04bec988).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class BadCodegenAndEvalExpression() extends LeafExpression `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737041791
**[Test build #132025 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132025/testReport)** for PR 30560 at commit [`6570d70`](https://github.com/apache/spark/commit/6570d70fd5416e5d39bbcf9b68e552ac66ebc8ff).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737042348
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737193189
**[Test build #132047 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132047/testReport)** for PR 30560 at commit [`95bdd37`](https://github.com/apache/spark/commit/95bdd379d7addcac25de3e52dd27710f04bec988).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
leanken commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737102496
> and
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-736994714
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737292862
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #30560: [SPARK-33619][SQL] GetMapValueUtil code generation error.
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533443749
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
##########
@@ -394,7 +394,7 @@ trait GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
val keyJavaType = CodeGenerator.javaType(keyType)
nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
val keyNotFoundBranch = if (failOnError) {
- s"""throw new NoSuchElementException("Key " + $eval2 + " does not exist.");"""
+ s"""throw new java.util.NoSuchElementException("Key " + $eval2 + " does not exist.");"""
Review comment:
> And as i investigated, there are some cases would fail if I correct the behavior of checkExceptionInExpression by using withSQLConf set CodeGenFallBack to false, like ObjectExpressionsSuite and RegexpExpressionsSuite and etc.
How about making a PR to fix `checkExceptionInExpression` first? I'm currently not sure how many failures caused by it exist now. If there are too many failures, we can separate work to fix them then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken edited a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
leanken edited a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737018589
> ```
> org.apache.spark.sql.catalyst.expressions.DateExpressionsSuite.Consistent error handling for datetime formatting and parsing functions 16 ms 1
> org.apache.spark.sql.catalyst.expressions.ObjectExpressionsSuite.SPARK-23594 GetExternalRowField should support interpreted execution 49 ms 1
> org.apache.spark.sql.catalyst.expressions.ObjectExpressionsSuite.SPARK-23595 ValidateExternalType should support interpreted execution 0.64 sec 1
> org.apache.spark.sql.catalyst.expressions.RegexpExpressionsSuite.RegexExtract 0.25 sec 1
> org.apache.spark.sql.catalyst.expressions.RegexpExpressionsSuite.RegexExtractAll
> ```
>
> It seems the five errors? If so, IMO its okay to fix all of them at once. Anyway, nice catch!
it should be zero error after the final patch 16940ce. please ignore the former test result ^_^
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-736983684
**[Test build #132019 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132019/testReport)** for PR 30560 at commit [`8e4ca3d`](https://github.com/apache/spark/commit/8e4ca3d4f9513506c57d0aecd2d46ca8878c1077).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
leanken commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737133597
> Could you update the title and the description, too?
update desc and add another UT in ExpressionEvalHelperSuite to make sure the `checkExceptionInExpression` change work as expect.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533951436
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
##########
@@ -160,9 +159,19 @@ trait ExpressionEvalHelper extends ScalaCheckDrivenPropertyChecks with PlanTestB
expectedErrMsg: String): Unit = {
def checkException(eval: => Unit, testMode: String): Unit = {
+ val modes = if (testMode == "non-codegen mode") {
+ Seq(CodegenObjectFactoryMode.NO_CODEGEN)
+ } else {
+ Seq(CodegenObjectFactoryMode.CODEGEN_ONLY, CodegenObjectFactoryMode.NO_CODEGEN)
Review comment:
For simplicity, can we always test it with 2 modes?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
cloud-fan removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737168166
is SparkR broken in the master branch? cc @HyukjinKwon
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737345790
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on a change in pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
leanken commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533909543
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
##########
@@ -195,17 +204,7 @@ trait ExpressionEvalHelper extends ScalaCheckDrivenPropertyChecks with PlanTestB
protected def generateProject(
Review comment:
should remove the entire function, but here we run an entire test to see if there are regressions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #30560: [SPARK-33619][SQL] GetMapValueUtil code generation error.
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533443749
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
##########
@@ -394,7 +394,7 @@ trait GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
val keyJavaType = CodeGenerator.javaType(keyType)
nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
val keyNotFoundBranch = if (failOnError) {
- s"""throw new NoSuchElementException("Key " + $eval2 + " does not exist.");"""
+ s"""throw new java.util.NoSuchElementException("Key " + $eval2 + " does not exist.");"""
Review comment:
> And as i investigated, there are some cases would fail if I correct the behavior of checkExceptionInExpression by using withSQLConf set CodeGenFallBack to false, like ObjectExpressionsSuite and RegexpExpressionsSuite and etc.
How about making a PR to fix `checkExceptionInExpression` first? I'm currently not sure how many failures caused by it exist. If there are too many failures, we can separate the work to fix them then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on a change in pull request #30560: [SPARK-33619][SQL] GetMapValueUtil code generation error.
Posted by GitBox <gi...@apache.org>.
leanken commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533303435
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
##########
@@ -394,7 +394,7 @@ trait GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
val keyJavaType = CodeGenerator.javaType(keyType)
nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
val keyNotFoundBranch = if (failOnError) {
- s"""throw new NoSuchElementException("Key " + $eval2 + " does not exist.");"""
+ s"""throw new java.util.NoSuchElementException("Key " + $eval2 + " does not exist.");"""
Review comment:
> @leanken, can we add a test?
Currently, this case being missed in SPARK-33460 it is because checkExceptionInExpression is not as expected. And as i investigated, there are some cases would fail if I correct the behavior of checkExceptionInExpression by using withSQLConf set CodeGenFallBack to false, like ObjectExpressionsSuite and RegexpExpressionsSuite and ETC.
So I need your advise. Should I
1. Fix all these wrong behavior separately and then final fix checkExceptionInExpression.
2. Fix all these wrong behavior and checkExceptionInExpression with the same patch.
The reason why i submit this PR it's because this bug introduced by me and I know of the history, but all those other behavior is fresh to me, and the all-in-one fix might be big.
As for add a test to the java.util.NoSuchElementException, I think the offline manual verification is enough, If we fix checkExceptionInExpression, and make it reliable, then we don't need to check code gen behavior for every new added or updated operation, right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737016161
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533646575
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
##########
@@ -394,7 +394,7 @@ trait GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
val keyJavaType = CodeGenerator.javaType(keyType)
nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
val keyNotFoundBranch = if (failOnError) {
- s"""throw new NoSuchElementException("Key " + $eval2 + " does not exist.");"""
+ s"""throw new java.util.NoSuchElementException("Key " + $eval2 + " does not exist.");"""
Review comment:
+1 for @maropu 's advice.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
leanken commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-736952405
let's see how many error occurs after I change the `checkExceptionInExpression `
@maropu @HyukjinKwon @dongjoon-hyun @cloud-fan
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737167724
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on pull request #30560: [SPARK-33619] GetMapValueUtil code generation error.
Posted by GitBox <gi...@apache.org>.
leanken commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-736348346
Why were
1. checkExceptionInExpression[Exception](expr, errMsg)
2. sql/testOnly org.apache.spark.sql.SQLQueryTestSuite – -z ansi/map.sql
in SPARK-33460 can't detect this Bug
It's because
1. checkExceptionInExpression is some how with error, too, but i don't know of the history. It should wrap with
withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key like CheckEvalulation, AND WE SHOULD FIX this later.
2. SQLQueryTestSuite ansi/map.sql failed to detect because of the Constant Folding rules, it is calling eval instead of code gen in this case
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-736985369
**[Test build #132019 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132019/testReport)** for PR 30560 at commit [`8e4ca3d`](https://github.com/apache/spark/commit/8e4ca3d4f9513506c57d0aecd2d46ca8878c1077).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-736960640
**[Test build #132015 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132015/testReport)** for PR 30560 at commit [`6d4fb1d`](https://github.com/apache/spark/commit/6d4fb1d287965b1123d204f9a98b31dd249c1617).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-736960640
**[Test build #132015 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132015/testReport)** for PR 30560 at commit [`6d4fb1d`](https://github.com/apache/spark/commit/6d4fb1d287965b1123d204f9a98b31dd249c1617).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on a change in pull request #30560: [SPARK-33619][SQL] GetMapValueUtil code generation error.
Posted by GitBox <gi...@apache.org>.
leanken commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533303435
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
##########
@@ -394,7 +394,7 @@ trait GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
val keyJavaType = CodeGenerator.javaType(keyType)
nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
val keyNotFoundBranch = if (failOnError) {
- s"""throw new NoSuchElementException("Key " + $eval2 + " does not exist.");"""
+ s"""throw new java.util.NoSuchElementException("Key " + $eval2 + " does not exist.");"""
Review comment:
> @leanken, can we add a test?
Currently, this case being missed in SPARK-33460 it is because checkExceptionInExpression is not as expected. And as i investigated, there are some cases would fail if I correct the behavior of checkExceptionInExpression by using withSQLConf set CodeGenFallBack to false, like ObjectExpressionsSuite and RegexpExpressionsSuite and etc.
So I need your advise. Should I
1. Fix all these wrong behavior separately and then finally fix checkExceptionInExpression.
2. Fix all these wrong behavior and checkExceptionInExpression with the same patch.
The reason why i submit this PR it's because this bug introduced by me and I know of the history, but all those other behavior is fresh to me, and the all-in-one fix might be big.
As for add a test to the java.util.NoSuchElementException, I think the offline manual verification is enough, If we fix checkExceptionInExpression, and make it reliable, then we don't need to check code gen behavior for every new added or updated operation, right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on a change in pull request #30560: [SPARK-33619][SQL] GetMapValueUtil code generation error.
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533443749
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
##########
@@ -394,7 +394,7 @@ trait GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
val keyJavaType = CodeGenerator.javaType(keyType)
nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
val keyNotFoundBranch = if (failOnError) {
- s"""throw new NoSuchElementException("Key " + $eval2 + " does not exist.");"""
+ s"""throw new java.util.NoSuchElementException("Key " + $eval2 + " does not exist.");"""
Review comment:
> And as i investigated, there are some cases would fail if I correct the behavior of checkExceptionInExpression by using withSQLConf set CodeGenFallBack to false, like ObjectExpressionsSuite and RegexpExpressionsSuite and etc.
How about making a PR to fix `checkExceptionInExpression` first? I'm not sure how many failures caused by it exist now. If there are too many failures, we can separate the work to fix them then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737225978
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #30560: [SPARK-33619] GetMapValueUtil code generation error.
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533265531
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
##########
@@ -394,7 +394,7 @@ trait GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
val keyJavaType = CodeGenerator.javaType(keyType)
nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
val keyNotFoundBranch = if (failOnError) {
- s"""throw new NoSuchElementException("Key " + $eval2 + " does not exist.");"""
+ s"""throw new java.util.NoSuchElementException("Key " + $eval2 + " does not exist.");"""
Review comment:
@leanken, can we add a test?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737171273
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on a change in pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
leanken commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533952098
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
##########
@@ -160,9 +159,19 @@ trait ExpressionEvalHelper extends ScalaCheckDrivenPropertyChecks with PlanTestB
expectedErrMsg: String): Unit = {
def checkException(eval: => Unit, testMode: String): Unit = {
+ val modes = if (testMode == "non-codegen mode") {
+ Seq(CodegenObjectFactoryMode.NO_CODEGEN)
+ } else {
+ Seq(CodegenObjectFactoryMode.CODEGEN_ONLY, CodegenObjectFactoryMode.NO_CODEGEN)
Review comment:
should be OK. since setting mode does not affect `evaluateWithoutCodegen`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on pull request #30560: [SPARK-33619] GetMapValueUtil code generation error.
Posted by GitBox <gi...@apache.org>.
leanken commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-736349801
@cloud-fan @HyukjinKwon FYI.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #30560:
URL: https://github.com/apache/spark/pull/30560
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737133458
**[Test build #132040 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132040/testReport)** for PR 30560 at commit [`95bdd37`](https://github.com/apache/spark/commit/95bdd379d7addcac25de3e52dd27710f04bec988).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-755062519
@HyukjinKwon @dongjoon-hyun I will give a pr try to fix this, It seems that there is some wrong with the generated code. But I am not sure why this pr triggered the problem ....
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737166674
**[Test build #132025 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132025/testReport)** for PR 30560 at commit [`6570d70`](https://github.com/apache/spark/commit/6570d70fd5416e5d39bbcf9b68e552ac66ebc8ff).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737076158
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737016161
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737000928
**[Test build #132022 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132022/testReport)** for PR 30560 at commit [`16940ce`](https://github.com/apache/spark/commit/16940ce89477f1c3839fed13e67b39792cb55fa6).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-736994714
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737041582
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737041791
**[Test build #132025 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132025/testReport)** for PR 30560 at commit [`6570d70`](https://github.com/apache/spark/commit/6570d70fd5416e5d39bbcf9b68e552ac66ebc8ff).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737041907
**[Test build #132022 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132022/testReport)** for PR 30560 at commit [`16940ce`](https://github.com/apache/spark/commit/16940ce89477f1c3839fed13e67b39792cb55fa6).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737041582
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737177778
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737168166
is SparkR broken in the master branch? cc @HyukjinKwon
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737345790
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-754281261
Hi, All.
This seems to break Scala 2.13 UT. Please see https://issues.apache.org/jira/browse/SPARK-33948?focusedCommentId=17258217&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17258217 .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737024489
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737225978
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737076158
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
leanken commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737618860
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737171273
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] maropu commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737078559
Could you update the title and the description, too?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737292862
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on a change in pull request #30560: [SPARK-33619][SQL] GetMapValueUtil code generation error.
Posted by GitBox <gi...@apache.org>.
leanken commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533306977
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
##########
@@ -394,7 +394,7 @@ trait GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
val keyJavaType = CodeGenerator.javaType(keyType)
nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
val keyNotFoundBranch = if (failOnError) {
- s"""throw new NoSuchElementException("Key " + $eval2 + " does not exist.");"""
+ s"""throw new java.util.NoSuchElementException("Key " + $eval2 + " does not exist.");"""
Review comment:
> @leanken, can we add a test?
maybe you can try reproduce ObjectExpressionsSuite and RegexpExpressionsSuite in your environment so that you can get clearer picture.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737193189
**[Test build #132047 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132047/testReport)** for PR 30560 at commit [`95bdd37`](https://github.com/apache/spark/commit/95bdd379d7addcac25de3e52dd27710f04bec988).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737331282
thanks, merging to master!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737004775
**[Test build #132015 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132015/testReport)** for PR 30560 at commit [`6d4fb1d`](https://github.com/apache/spark/commit/6d4fb1d287965b1123d204f9a98b31dd249c1617).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class MakeDate(`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737344321
**[Test build #132047 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132047/testReport)** for PR 30560 at commit [`95bdd37`](https://github.com/apache/spark/commit/95bdd379d7addcac25de3e52dd27710f04bec988).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class BadCodegenAndEvalExpression() extends LeafExpression `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737133458
**[Test build #132040 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132040/testReport)** for PR 30560 at commit [`95bdd37`](https://github.com/apache/spark/commit/95bdd379d7addcac25de3e52dd27710f04bec988).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737000928
**[Test build #132022 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/132022/testReport)** for PR 30560 at commit [`16940ce`](https://github.com/apache/spark/commit/16940ce89477f1c3839fed13e67b39792cb55fa6).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on a change in pull request #30560: [SPARK-33619][SQL] GetMapValueUtil code generation error.
Posted by GitBox <gi...@apache.org>.
leanken commented on a change in pull request #30560:
URL: https://github.com/apache/spark/pull/30560#discussion_r533306977
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/complexTypeExtractors.scala
##########
@@ -394,7 +394,7 @@ trait GetMapValueUtil extends BinaryExpression with ImplicitCastInputTypes {
val keyJavaType = CodeGenerator.javaType(keyType)
nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
val keyNotFoundBranch = if (failOnError) {
- s"""throw new NoSuchElementException("Key " + $eval2 + " does not exist.");"""
+ s"""throw new java.util.NoSuchElementException("Key " + $eval2 + " does not exist.");"""
Review comment:
> @leanken, can we add a test?
maybe you can try repo ObjectExpressionsSuite and RegexpExpressionsSuite in your environment so that you can get clearer picture.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] leanken commented on pull request #30560: [SPARK-33619][SQL] Fix GetMapValueUtil code generation error
Posted by GitBox <gi...@apache.org>.
leanken commented on pull request #30560:
URL: https://github.com/apache/spark/pull/30560#issuecomment-737018589
> ```
> org.apache.spark.sql.catalyst.expressions.DateExpressionsSuite.Consistent error handling for datetime formatting and parsing functions 16 ms 1
> org.apache.spark.sql.catalyst.expressions.ObjectExpressionsSuite.SPARK-23594 GetExternalRowField should support interpreted execution 49 ms 1
> org.apache.spark.sql.catalyst.expressions.ObjectExpressionsSuite.SPARK-23595 ValidateExternalType should support interpreted execution 0.64 sec 1
> org.apache.spark.sql.catalyst.expressions.RegexpExpressionsSuite.RegexExtract 0.25 sec 1
> org.apache.spark.sql.catalyst.expressions.RegexpExpressionsSuite.RegexExtractAll
> ```
>
> It seems the five errors? If so, IMO its okay to fix all of them at once. Anyway, nice catch!
it should be zero error after the final patch. please ignore the former test result ^_^
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org