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 2021/03/06 20:50:10 UTC
[GitHub] [spark] dongjoon-hyun opened a new pull request #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
dongjoon-hyun opened a new pull request #31766:
URL: https://github.com/apache/spark/pull/31766
### What changes were proposed in this pull request?
This PR aims to add `testFallback` function to `PlanTest`.
### Why are the changes needed?
Some test cases only work in `CodegenObjectFactoryMode.FALLBACK` mode while `PlanTest` doesn't support it because it always overrides the test case and run twice with `CODEGEN_ONLY` and `NO_CODEGEN`.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Pass the CIs.
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796131243
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795133993
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40511/
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r591201013
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -51,6 +51,15 @@ trait CodegenInterpretedPlanTest extends PlanTest {
super.test(testName + " (interpreted path)", testTags: _*)(testFun)(pos)
}
}
+
+ protected def testFallback(
+ testName: String,
+ testTags: Tag*)(testFun: => Any)(implicit pos: source.Position): Unit = {
+ val codegenMode = CodegenObjectFactoryMode.FALLBACK.toString
+ withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
Review comment:
The fix LGTM. It's better to know why the test passed before this PR. The `test` above runs the test code with `withSQLConf` to enable codegen.
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795067158
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135930/
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795319695
**[Test build #135927 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135927/testReport)** for PR 31766 at commit [`d24be4c`](https://github.com/apache/spark/commit/d24be4c1eedaac935749c80b2e1b7e73697f3d70).
* This patch passes all 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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795335040
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135927/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796049091
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40528/
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795133993
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40511/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592083866
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -44,12 +44,18 @@ trait CodegenInterpretedPlanTest extends PlanTest {
val codegenMode = CodegenObjectFactoryMode.CODEGEN_ONLY.toString
val interpretedMode = CodegenObjectFactoryMode.NO_CODEGEN.toString
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
- super.test(testName + " (codegen path)", testTags: _*)(testFun)(pos)
Review comment:
When the derived classes define test cases with this `test`, two test cases are created but `withSQLConf` is not inside the defined tests.
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795335040
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135927/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796034340
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40526/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-797160335
late lgtm. Nice fix for making these hidden bugs clearer!
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796882473
Thank you, @kiszk .
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-792071348
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40418/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796388151
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40538/
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795097788
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40513/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796491808
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135954/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796020651
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40526/
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r591729833
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -51,6 +51,15 @@ trait CodegenInterpretedPlanTest extends PlanTest {
super.test(testName + " (interpreted path)", testTags: _*)(testFun)(pos)
}
}
+
+ protected def testFallback(
+ testName: String,
+ testTags: Tag*)(testFun: => Any)(implicit pos: source.Position): Unit = {
+ val codegenMode = CodegenObjectFactoryMode.FALLBACK.toString
+ withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
Review comment:
Actually, for that one, I had better to create a new JIRA. I'll open a new one 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] dongjoon-hyun edited a comment on pull request #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795018697
Hi, @rednaxelafx , @cloud-fan , @viirya , @kiszk , @maropu , @HyukjinKwon .
Here is a summary of the progress.
1. https://github.com/apache/spark/pull/31775 (SPARK-34660 `Don't use ParVector with withExistingConf which is not thread-safe`) fixed the UT failure at `branch-3.1` last weekend.
2. https://github.com/apache/spark/pull/31785 (SPARK-33498 `Remove SQLConf.withExistingConf in CastSuite`) removes the unneeded `withExistingConf` at `master` and will be backported to branch-3.1.
So, the above two were the answers why we hit the UT failure only at `branch-3.1` while we have the patch at `master/3.1/3.0`.
Now, this PR aims to do **only refactoring PlanTest and test cases to have a clear test design**. As we discussed in the previous PR, we need this.
- https://github.com/apache/spark/pull/31764#discussion_r589170423 (@cloud-fan )
> The current test framework assumes that codegen and non-codegen should have consistent behaviors, while this codegen bug breaks the assumption. The test case fails with codegen but passes with non-codegen.
>
> +1 to add this for such test cases. One thing I'm curious about is why this test works in master...
- https://github.com/apache/spark/pull/31764#discussion_r589179603 (@maropu )
> +1 to add it, too.
I updated this PR to the master and address the two comments.
- https://github.com/apache/spark/pull/31764#discussion_r589179924
- https://github.com/apache/spark/pull/31764#discussion_r589182718
So, could you review this again freshly? And, please let me know if I missed some of your comments, (especially @viirya 's one). I hope we can have this refactoring for all branches with SPARK-34596 and SPARK-34607.
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795968522
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135944/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592093684
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
##########
@@ -161,15 +161,16 @@ class ExpressionEncoderSuite extends CodegenInterpretedPlanTest with AnalysisTes
encodeDecodeTest(Seq(Seq("abc", "xyz"), Seq[String](null), null, Seq("1", null, "2")),
"seq of seq of string")
- encodeDecodeTest(Array(31, -123, 4), "array of int")
+ encodeDecodeTest(Array(31, -123, 4), "array of int", useFallback = true)
Review comment:
I also was surprised that we have several cases instead of the new two tests.
It seems to be broken some commits during the time because we didn't notice it due to `CodegenInterpretedPlanTest` bug.
Actually, I decided to focus on the test suite first and makes the recovery is the beyond of the scope of this PR.
We may had better file a new JIRA aiming the recovery of those test cases one by one if possible.
BTW, for this specific instance, the following was the error.
```
[info] - encode/decode for array of int: [I@74655d03 (interpreted path) *** FAILED *** (14 milliseconds)
[info] Exception thrown while decoding
[info] Converted: [0,1000000020,3,0,ffffff850000001f,4]
[info] Schema: value#680
[info] root
[info] -- value: array (nullable = true)
[info] |-- element: integer (containsNull = false)
[info]
[info]
[info] Encoder:
[info] class[value[0]: array<int>] (ExpressionEncoderSuite.scala:578)
[info] org.scalatest.exceptions.TestFailedException:
[info] at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
[info] at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471)
[info] at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1563)
[info] at org.scalatest.Assertions.fail(Assertions.scala:949)
[info] at org.scalatest.Assertions.fail$(Assertions.scala:945)
[info] at org.scalatest.funsuite.AnyFunSuite.fail(AnyFunSuite.scala:1563)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$encodeDecodeTest$1(ExpressionEncoderSuite.scala:578)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.verifyNotLeakingReflectionObjects(ExpressionEncoderSuite.scala:656)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$testAndVerifyNotLeakingReflectionObjects$2(ExpressionEncoderSuite.scala:669)
[info] at org.apache.spark.sql.catalyst.plans.CodegenInterpretedPlanTest.$anonfun$test$4(PlanTest.scala:50)
[info] at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf(SQLHelper.scala:54)
[info] at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf$(SQLHelper.scala:38)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.withSQLConf(ExpressionEncoderSuite.scala:118)
[info] at org.apache.spark.sql.catalyst.plans.CodegenInterpretedPlanTest.$anonfun$test$3(PlanTest.scala:50)
[info] at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
[info] at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
[info] at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
[info] at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
[info] at org.scalatest.Transformer.apply(Transformer.scala:22)
[info] at org.scalatest.Transformer.apply(Transformer.scala:20)
[info] at org.scalatest.funsuite.AnyFunSuiteLike$$anon$1.apply(AnyFunSuiteLike.scala:190)
[info] at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:178)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.invokeWithFixture$1(AnyFunSuiteLike.scala:188)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTest$1(AnyFunSuiteLike.scala:200)
[info] at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTest(AnyFunSuiteLike.scala:200)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTest$(AnyFunSuiteLike.scala:182)
[info] at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterEach$$super$runTest(SparkFunSuite.scala:61)
[info] at org.scalatest.BeforeAndAfterEach.runTest(BeforeAndAfterEach.scala:234)
[info] at org.scalatest.BeforeAndAfterEach.runTest$(BeforeAndAfterEach.scala:227)
[info] at org.apache.spark.SparkFunSuite.runTest(SparkFunSuite.scala:61)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTests$1(AnyFunSuiteLike.scala:233)
[info] at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:413)
[info] at scala.collection.immutable.List.foreach(List.scala:392)
[info] at org.scalatest.SuperEngine.traverseSubNodes$1(Engine.scala:401)
[info] at org.scalatest.SuperEngine.runTestsInBranch(Engine.scala:396)
[info] at org.scalatest.SuperEngine.runTestsImpl(Engine.scala:475)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTests(AnyFunSuiteLike.scala:233)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTests$(AnyFunSuiteLike.scala:232)
[info] at org.scalatest.funsuite.AnyFunSuite.runTests(AnyFunSuite.scala:1563)
[info] at org.scalatest.Suite.run(Suite.scala:1112)
[info] at org.scalatest.Suite.run$(Suite.scala:1094)
[info] at org.scalatest.funsuite.AnyFunSuite.org$scalatest$funsuite$AnyFunSuiteLike$$super$run(AnyFunSuite.scala:1563)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$run$1(AnyFunSuiteLike.scala:237)
[info] at org.scalatest.SuperEngine.runImpl(Engine.scala:535)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.run(AnyFunSuiteLike.scala:237)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.run$(AnyFunSuiteLike.scala:236)
[info] at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterAll$$super$run(SparkFunSuite.scala:61)
[info] at org.scalatest.BeforeAndAfterAll.liftedTree1$1(BeforeAndAfterAll.scala:213)
[info] at org.scalatest.BeforeAndAfterAll.run(BeforeAndAfterAll.scala:210)
[info] at org.scalatest.BeforeAndAfterAll.run$(BeforeAndAfterAll.scala:208)
[info] at org.apache.spark.SparkFunSuite.run(SparkFunSuite.scala:61)
[info] at org.scalatest.tools.Framework.org$scalatest$tools$Framework$$runSuite(Framework.scala:318)
[info] at org.scalatest.tools.Framework$ScalaTestTask.execute(Framework.scala:513)
[info] at sbt.ForkMain$Run.lambda$runTest$1(ForkMain.java:413)
[info] at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[info] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[info] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[info] at java.lang.Thread.run(Thread.java:748)
[info] Cause: java.lang.RuntimeException: Error while decoding: java.lang.NoSuchMethodException: org.apache.spark.sql.catalyst.util.GenericArrayData.toIntArray()
[info] mapobjects(lambdavariable(MapObject, IntegerType, false, -1), assertnotnull(lambdavariable(MapObject, IntegerType, false, -1)), input[0, array<int>, true], None).toIntArray
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder$Deserializer.apply(ExpressionEncoder.scala:186)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$encodeDecodeTest$1(ExpressionEncoderSuite.scala:576)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.verifyNotLeakingReflectionObjects(ExpressionEncoderSuite.scala:656)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$testAndVerifyNotLeakingReflectionObjects$2(ExpressionEncoderSuite.scala:669)
```
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-792157707
**[Test build #135836 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135836/testReport)** for PR 31766 at commit [`2a2e01c`](https://github.com/apache/spark/commit/2a2e01c39e29c0233ba6c9e7c30ff35b2cf6a583).
* This patch passes all 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] SparkQA commented on pull request #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796404058
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40538/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796467837
**[Test build #135954 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135954/testReport)** for PR 31766 at commit [`54636fc`](https://github.com/apache/spark/commit/54636fce7de8368d23f7133d862ea737845954b1).
* This patch passes all 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] dongjoon-hyun edited a comment on pull request #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795018697
Hi, @rednaxelafx , @cloud-fan , @viirya , @kiszk , @maropu , @HyukjinKwon .
Here is a summary of the progress.
1. https://github.com/apache/spark/pull/31775 (SPARK-34660 `Don't use ParVector with withExistingConf which is not thread-safe`) fixed the UT failure at `branch-3.1` last weekend.
2. https://github.com/apache/spark/pull/31785 (SPARK-33498 `Remove SQLConf.withExistingConf in CastSuite`) removes the unneeded `withExistingConf` at `master` and will be backported to branch-3.1.
So, the above two were the answers why we hit the UT failure only at `branch-3.1` while we have the patch at `master/3.1/3.0`.
Now, this PR aims to do **only refactoring PlanTest and test cases to have a clear test design**. As we discussed in the previous PR, we need this.
- https://github.com/apache/spark/pull/31764#discussion_r589170423 (@cloud-fan )
> The current test framework assumes that codegen and non-codegen should have consistent behaviors, while this codegen bug breaks the assumption. The test case fails with codegen but passes with non-codegen.
>
> +1 to add this for such test cases. One thing I'm curious about is why this test works in master...
- https://github.com/apache/spark/pull/31764#discussion_r589179603 (@maropu )
> +1 to add it, too.
I updated this PR to the master and address the two comments.
- https://github.com/apache/spark/pull/31764#discussion_r589179924
- https://github.com/apache/spark/pull/31764#discussion_r589182718
So, could you review this again freshly? And, please let me know if I missed some of your comments, (especially @viirya 's one).
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592082864
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -44,12 +44,18 @@ trait CodegenInterpretedPlanTest extends PlanTest {
val codegenMode = CodegenObjectFactoryMode.CODEGEN_ONLY.toString
val interpretedMode = CodegenObjectFactoryMode.NO_CODEGEN.toString
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
- super.test(testName + " (codegen path)", testTags: _*)(testFun)(pos)
Review comment:
Here, we define these tests by using `super.test` instead of executing.
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796086747
**[Test build #135942 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135942/testReport)** for PR 31766 at commit [`0b2915d`](https://github.com/apache/spark/commit/0b2915d969c584667766769a0a7109db18f2d9bb).
* 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] SparkQA removed a comment on pull request #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-792058284
**[Test build #135836 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135836/testReport)** for PR 31766 at commit [`2a2e01c`](https://github.com/apache/spark/commit/2a2e01c39e29c0233ba6c9e7c30ff35b2cf6a583).
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795085026
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40511/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795901120
**[Test build #135942 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135942/testReport)** for PR 31766 at commit [`0b2915d`](https://github.com/apache/spark/commit/0b2915d969c584667766769a0a7109db18f2d9bb).
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796120344
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40528/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592094966
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
##########
@@ -161,15 +161,16 @@ class ExpressionEncoderSuite extends CodegenInterpretedPlanTest with AnalysisTes
encodeDecodeTest(Seq(Seq("abc", "xyz"), Seq[String](null), null, Seq("1", null, "2")),
"seq of seq of string")
- encodeDecodeTest(Array(31, -123, 4), "array of int")
+ encodeDecodeTest(Array(31, -123, 4), "array of int", useFallback = true)
Review comment:
This test suite fix aims to land to `master/3.1/3.0/2.4`.
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795968522
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135944/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592093684
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
##########
@@ -161,15 +161,16 @@ class ExpressionEncoderSuite extends CodegenInterpretedPlanTest with AnalysisTes
encodeDecodeTest(Seq(Seq("abc", "xyz"), Seq[String](null), null, Seq("1", null, "2")),
"seq of seq of string")
- encodeDecodeTest(Array(31, -123, 4), "array of int")
+ encodeDecodeTest(Array(31, -123, 4), "array of int", useFallback = true)
Review comment:
I also was surprised that we have several cases instead of the new two tests.
It seems to be broken some commits during the time because we didn't notice it due to this `CodegenInterpretedPlanTest` test framework bug.
Actually, I decided to focus on the test suite first and makes the recovery the beyond of the scope of this PR.
We may had better file a new JIRA aiming the recovery of those test cases one by one if possible.
BTW, for this specific instance, the following was the error.
```
[info] - encode/decode for array of int: [I@74655d03 (interpreted path) *** FAILED *** (14 milliseconds)
[info] Exception thrown while decoding
[info] Converted: [0,1000000020,3,0,ffffff850000001f,4]
[info] Schema: value#680
[info] root
[info] -- value: array (nullable = true)
[info] |-- element: integer (containsNull = false)
[info]
[info]
[info] Encoder:
[info] class[value[0]: array<int>] (ExpressionEncoderSuite.scala:578)
[info] org.scalatest.exceptions.TestFailedException:
[info] at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
[info] at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471)
[info] at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1563)
[info] at org.scalatest.Assertions.fail(Assertions.scala:949)
[info] at org.scalatest.Assertions.fail$(Assertions.scala:945)
[info] at org.scalatest.funsuite.AnyFunSuite.fail(AnyFunSuite.scala:1563)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$encodeDecodeTest$1(ExpressionEncoderSuite.scala:578)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.verifyNotLeakingReflectionObjects(ExpressionEncoderSuite.scala:656)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$testAndVerifyNotLeakingReflectionObjects$2(ExpressionEncoderSuite.scala:669)
[info] at org.apache.spark.sql.catalyst.plans.CodegenInterpretedPlanTest.$anonfun$test$4(PlanTest.scala:50)
[info] at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf(SQLHelper.scala:54)
[info] at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf$(SQLHelper.scala:38)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.withSQLConf(ExpressionEncoderSuite.scala:118)
[info] at org.apache.spark.sql.catalyst.plans.CodegenInterpretedPlanTest.$anonfun$test$3(PlanTest.scala:50)
[info] at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
[info] at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
[info] at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
[info] at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
[info] at org.scalatest.Transformer.apply(Transformer.scala:22)
[info] at org.scalatest.Transformer.apply(Transformer.scala:20)
[info] at org.scalatest.funsuite.AnyFunSuiteLike$$anon$1.apply(AnyFunSuiteLike.scala:190)
[info] at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:178)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.invokeWithFixture$1(AnyFunSuiteLike.scala:188)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTest$1(AnyFunSuiteLike.scala:200)
[info] at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTest(AnyFunSuiteLike.scala:200)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTest$(AnyFunSuiteLike.scala:182)
[info] at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterEach$$super$runTest(SparkFunSuite.scala:61)
[info] at org.scalatest.BeforeAndAfterEach.runTest(BeforeAndAfterEach.scala:234)
[info] at org.scalatest.BeforeAndAfterEach.runTest$(BeforeAndAfterEach.scala:227)
[info] at org.apache.spark.SparkFunSuite.runTest(SparkFunSuite.scala:61)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTests$1(AnyFunSuiteLike.scala:233)
[info] at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:413)
[info] at scala.collection.immutable.List.foreach(List.scala:392)
[info] at org.scalatest.SuperEngine.traverseSubNodes$1(Engine.scala:401)
[info] at org.scalatest.SuperEngine.runTestsInBranch(Engine.scala:396)
[info] at org.scalatest.SuperEngine.runTestsImpl(Engine.scala:475)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTests(AnyFunSuiteLike.scala:233)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTests$(AnyFunSuiteLike.scala:232)
[info] at org.scalatest.funsuite.AnyFunSuite.runTests(AnyFunSuite.scala:1563)
[info] at org.scalatest.Suite.run(Suite.scala:1112)
[info] at org.scalatest.Suite.run$(Suite.scala:1094)
[info] at org.scalatest.funsuite.AnyFunSuite.org$scalatest$funsuite$AnyFunSuiteLike$$super$run(AnyFunSuite.scala:1563)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$run$1(AnyFunSuiteLike.scala:237)
[info] at org.scalatest.SuperEngine.runImpl(Engine.scala:535)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.run(AnyFunSuiteLike.scala:237)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.run$(AnyFunSuiteLike.scala:236)
[info] at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterAll$$super$run(SparkFunSuite.scala:61)
[info] at org.scalatest.BeforeAndAfterAll.liftedTree1$1(BeforeAndAfterAll.scala:213)
[info] at org.scalatest.BeforeAndAfterAll.run(BeforeAndAfterAll.scala:210)
[info] at org.scalatest.BeforeAndAfterAll.run$(BeforeAndAfterAll.scala:208)
[info] at org.apache.spark.SparkFunSuite.run(SparkFunSuite.scala:61)
[info] at org.scalatest.tools.Framework.org$scalatest$tools$Framework$$runSuite(Framework.scala:318)
[info] at org.scalatest.tools.Framework$ScalaTestTask.execute(Framework.scala:513)
[info] at sbt.ForkMain$Run.lambda$runTest$1(ForkMain.java:413)
[info] at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[info] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[info] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[info] at java.lang.Thread.run(Thread.java:748)
[info] Cause: java.lang.RuntimeException: Error while decoding: java.lang.NoSuchMethodException: org.apache.spark.sql.catalyst.util.GenericArrayData.toIntArray()
[info] mapobjects(lambdavariable(MapObject, IntegerType, false, -1), assertnotnull(lambdavariable(MapObject, IntegerType, false, -1)), input[0, array<int>, true], None).toIntArray
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder$Deserializer.apply(ExpressionEncoder.scala:186)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$encodeDecodeTest$1(ExpressionEncoderSuite.scala:576)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.verifyNotLeakingReflectionObjects(ExpressionEncoderSuite.scala:656)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$testAndVerifyNotLeakingReflectionObjects$2(ExpressionEncoderSuite.scala:669)
```
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795126419
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40511/
----------------------------------------------------------------
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] viirya commented on a change in pull request #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r591761006
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -44,12 +44,18 @@ trait CodegenInterpretedPlanTest extends PlanTest {
val codegenMode = CodegenObjectFactoryMode.CODEGEN_ONLY.toString
val interpretedMode = CodegenObjectFactoryMode.NO_CODEGEN.toString
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
- super.test(testName + " (codegen path)", testTags: _*)(testFun)(pos)
- }
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> interpretedMode) {
- super.test(testName + " (interpreted path)", testTags: _*)(testFun)(pos)
- }
+ super.test(testName + " (codegen path)", testTags: _*)(
+ withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) { testFun })(pos)
+ super.test(testName + " (interpreted path)", testTags: _*)(
+ withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> interpretedMode) { testFun })(pos)
Review comment:
Ah, good catch! I was bitten by this before in other place. This was added more longer before. Thanks for catching and fixing it.
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795901120
**[Test build #135942 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135942/testReport)** for PR 31766 at commit [`0b2915d`](https://github.com/apache/spark/commit/0b2915d969c584667766769a0a7109db18f2d9bb).
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592075456
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -44,12 +44,18 @@ trait CodegenInterpretedPlanTest extends PlanTest {
val codegenMode = CodegenObjectFactoryMode.CODEGEN_ONLY.toString
val interpretedMode = CodegenObjectFactoryMode.NO_CODEGEN.toString
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
- super.test(testName + " (codegen path)", testTags: _*)(testFun)(pos)
Review comment:
`withSQLConf` depends on `SQLConf.get`. The existing code cannot guarantee the configuration setting at line 47 at `super.test`'s execution time.
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592084726
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -44,12 +44,18 @@ trait CodegenInterpretedPlanTest extends PlanTest {
val codegenMode = CodegenObjectFactoryMode.CODEGEN_ONLY.toString
val interpretedMode = CodegenObjectFactoryMode.NO_CODEGEN.toString
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
- super.test(testName + " (codegen path)", testTags: _*)(testFun)(pos)
Review comment:
ah, now I got it! What a hidden bug 😂
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592069995
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -44,12 +44,18 @@ trait CodegenInterpretedPlanTest extends PlanTest {
val codegenMode = CodegenObjectFactoryMode.CODEGEN_ONLY.toString
val interpretedMode = CodegenObjectFactoryMode.NO_CODEGEN.toString
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
- super.test(testName + " (codegen path)", testTags: _*)(testFun)(pos)
Review comment:
`super.test` sets the sql conf again?
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-792161877
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135836/
----------------------------------------------------------------
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] kiszk commented on pull request #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
kiszk commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796690449
Late LGTM, thank you!
----------------------------------------------------------------
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 closed pull request #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #31766:
URL: https://github.com/apache/spark/pull/31766
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592075456
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -44,12 +44,18 @@ trait CodegenInterpretedPlanTest extends PlanTest {
val codegenMode = CodegenObjectFactoryMode.CODEGEN_ONLY.toString
val interpretedMode = CodegenObjectFactoryMode.NO_CODEGEN.toString
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
- super.test(testName + " (codegen path)", testTags: _*)(testFun)(pos)
Review comment:
`withSQLConf` depends on `SQLConf.get`. The existing code cannot guarantee the configuration setting of line 47 at `super.test`'s execution time.
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-792161877
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135836/
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-792058284
**[Test build #135836 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135836/testReport)** for PR 31766 at commit [`2a2e01c`](https://github.com/apache/spark/commit/2a2e01c39e29c0233ba6c9e7c30ff35b2cf6a583).
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795067158
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135930/
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795037300
**[Test build #135927 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135927/testReport)** for PR 31766 at commit [`d24be4c`](https://github.com/apache/spark/commit/d24be4c1eedaac935749c80b2e1b7e73697f3d70).
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796409430
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40538/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795966161
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40526/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796409430
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40538/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796491808
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135954/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592093684
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
##########
@@ -161,15 +161,16 @@ class ExpressionEncoderSuite extends CodegenInterpretedPlanTest with AnalysisTes
encodeDecodeTest(Seq(Seq("abc", "xyz"), Seq[String](null), null, Seq("1", null, "2")),
"seq of seq of string")
- encodeDecodeTest(Array(31, -123, 4), "array of int")
+ encodeDecodeTest(Array(31, -123, 4), "array of int", useFallback = true)
Review comment:
I also was surprised that we have several cases instead of the new two tests.
It seems to be broken at some commits during the time because we didn't notice it due to this `CodegenInterpretedPlanTest` test framework bug.
Actually, I decided to focus on the test suite first and makes the recovery the beyond of the scope of this PR.
We may had better file a new JIRA aiming the recovery of those test cases one by one if possible.
BTW, for this specific instance, the following was the error.
```
[info] - encode/decode for array of int: [I@74655d03 (interpreted path) *** FAILED *** (14 milliseconds)
[info] Exception thrown while decoding
[info] Converted: [0,1000000020,3,0,ffffff850000001f,4]
[info] Schema: value#680
[info] root
[info] -- value: array (nullable = true)
[info] |-- element: integer (containsNull = false)
[info]
[info]
[info] Encoder:
[info] class[value[0]: array<int>] (ExpressionEncoderSuite.scala:578)
[info] org.scalatest.exceptions.TestFailedException:
[info] at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
[info] at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471)
[info] at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1563)
[info] at org.scalatest.Assertions.fail(Assertions.scala:949)
[info] at org.scalatest.Assertions.fail$(Assertions.scala:945)
[info] at org.scalatest.funsuite.AnyFunSuite.fail(AnyFunSuite.scala:1563)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$encodeDecodeTest$1(ExpressionEncoderSuite.scala:578)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.verifyNotLeakingReflectionObjects(ExpressionEncoderSuite.scala:656)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$testAndVerifyNotLeakingReflectionObjects$2(ExpressionEncoderSuite.scala:669)
[info] at org.apache.spark.sql.catalyst.plans.CodegenInterpretedPlanTest.$anonfun$test$4(PlanTest.scala:50)
[info] at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf(SQLHelper.scala:54)
[info] at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf$(SQLHelper.scala:38)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.withSQLConf(ExpressionEncoderSuite.scala:118)
[info] at org.apache.spark.sql.catalyst.plans.CodegenInterpretedPlanTest.$anonfun$test$3(PlanTest.scala:50)
[info] at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
[info] at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
[info] at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
[info] at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
[info] at org.scalatest.Transformer.apply(Transformer.scala:22)
[info] at org.scalatest.Transformer.apply(Transformer.scala:20)
[info] at org.scalatest.funsuite.AnyFunSuiteLike$$anon$1.apply(AnyFunSuiteLike.scala:190)
[info] at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:178)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.invokeWithFixture$1(AnyFunSuiteLike.scala:188)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTest$1(AnyFunSuiteLike.scala:200)
[info] at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTest(AnyFunSuiteLike.scala:200)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTest$(AnyFunSuiteLike.scala:182)
[info] at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterEach$$super$runTest(SparkFunSuite.scala:61)
[info] at org.scalatest.BeforeAndAfterEach.runTest(BeforeAndAfterEach.scala:234)
[info] at org.scalatest.BeforeAndAfterEach.runTest$(BeforeAndAfterEach.scala:227)
[info] at org.apache.spark.SparkFunSuite.runTest(SparkFunSuite.scala:61)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTests$1(AnyFunSuiteLike.scala:233)
[info] at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:413)
[info] at scala.collection.immutable.List.foreach(List.scala:392)
[info] at org.scalatest.SuperEngine.traverseSubNodes$1(Engine.scala:401)
[info] at org.scalatest.SuperEngine.runTestsInBranch(Engine.scala:396)
[info] at org.scalatest.SuperEngine.runTestsImpl(Engine.scala:475)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTests(AnyFunSuiteLike.scala:233)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTests$(AnyFunSuiteLike.scala:232)
[info] at org.scalatest.funsuite.AnyFunSuite.runTests(AnyFunSuite.scala:1563)
[info] at org.scalatest.Suite.run(Suite.scala:1112)
[info] at org.scalatest.Suite.run$(Suite.scala:1094)
[info] at org.scalatest.funsuite.AnyFunSuite.org$scalatest$funsuite$AnyFunSuiteLike$$super$run(AnyFunSuite.scala:1563)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$run$1(AnyFunSuiteLike.scala:237)
[info] at org.scalatest.SuperEngine.runImpl(Engine.scala:535)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.run(AnyFunSuiteLike.scala:237)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.run$(AnyFunSuiteLike.scala:236)
[info] at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterAll$$super$run(SparkFunSuite.scala:61)
[info] at org.scalatest.BeforeAndAfterAll.liftedTree1$1(BeforeAndAfterAll.scala:213)
[info] at org.scalatest.BeforeAndAfterAll.run(BeforeAndAfterAll.scala:210)
[info] at org.scalatest.BeforeAndAfterAll.run$(BeforeAndAfterAll.scala:208)
[info] at org.apache.spark.SparkFunSuite.run(SparkFunSuite.scala:61)
[info] at org.scalatest.tools.Framework.org$scalatest$tools$Framework$$runSuite(Framework.scala:318)
[info] at org.scalatest.tools.Framework$ScalaTestTask.execute(Framework.scala:513)
[info] at sbt.ForkMain$Run.lambda$runTest$1(ForkMain.java:413)
[info] at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[info] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[info] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[info] at java.lang.Thread.run(Thread.java:748)
[info] Cause: java.lang.RuntimeException: Error while decoding: java.lang.NoSuchMethodException: org.apache.spark.sql.catalyst.util.GenericArrayData.toIntArray()
[info] mapobjects(lambdavariable(MapObject, IntegerType, false, -1), assertnotnull(lambdavariable(MapObject, IntegerType, false, -1)), input[0, array<int>, true], None).toIntArray
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder$Deserializer.apply(ExpressionEncoder.scala:186)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$encodeDecodeTest$1(ExpressionEncoderSuite.scala:576)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.verifyNotLeakingReflectionObjects(ExpressionEncoderSuite.scala:656)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$testAndVerifyNotLeakingReflectionObjects$2(ExpressionEncoderSuite.scala:669)
```
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796034340
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40526/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592085063
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
##########
@@ -161,15 +161,16 @@ class ExpressionEncoderSuite extends CodegenInterpretedPlanTest with AnalysisTes
encodeDecodeTest(Seq(Seq("abc", "xyz"), Seq[String](null), null, Seq("1", null, "2")),
"seq of seq of string")
- encodeDecodeTest(Array(31, -123, 4), "array of int")
+ encodeDecodeTest(Array(31, -123, 4), "array of int", useFallback = true)
Review comment:
do we know why this simply test has to use fallback 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 commented on pull request #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796131243
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796350884
**[Test build #135954 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135954/testReport)** for PR 31766 at commit [`54636fc`](https://github.com/apache/spark/commit/54636fce7de8368d23f7133d862ea737845954b1).
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-792071348
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40418/
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795137990
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40513/
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795137990
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40513/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796337550
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] SparkQA removed a comment on pull request #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795037300
**[Test build #135927 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135927/testReport)** for PR 31766 at commit [`d24be4c`](https://github.com/apache/spark/commit/d24be4c1eedaac935749c80b2e1b7e73697f3d70).
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796350884
**[Test build #135954 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135954/testReport)** for PR 31766 at commit [`54636fc`](https://github.com/apache/spark/commit/54636fce7de8368d23f7133d862ea737845954b1).
----------------------------------------------------------------
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 edited a comment on pull request #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795018697
Hi, @rednaxelafx , @cloud-fan , @viirya , @kiszk , @maropu , @HyukjinKwon .
Here is a summary of the progress.
1. https://github.com/apache/spark/pull/31775 (SPARK-34660 `Don't use ParVector with withExistingConf which is not thread-safe`) fixed the UT failure at `branch-3.1` last weekend.
2. https://github.com/apache/spark/pull/31785 (SPARK-33498 `Remove SQLConf.withExistingConf in CastSuite`) removes the unneeded `withExistingConf` at `master` and will be backported to branch-3.1.
So, the above two were the answer why we hit the UT failure only at `branch-3.1` while we have the patch at `master/3.1/3.0`.
Now, this PR aims to do **only refactoring PlanTest and test cases to have a clear test design**. As we discussed in the previous PR, we need this.
- https://github.com/apache/spark/pull/31764#discussion_r589170423 (@cloud-fan )
> The current test framework assumes that codegen and non-codegen should have consistent behaviors, while this codegen bug breaks the assumption. The test case fails with codegen but passes with non-codegen.
>
> +1 to add this for such test cases. One thing I'm curious about is why this test works in master...
- https://github.com/apache/spark/pull/31764#discussion_r589179603 (@maropu )
> +1 to add it, too.
I updated this PR to the master and address the two comments.
- https://github.com/apache/spark/pull/31764#discussion_r589179924
- https://github.com/apache/spark/pull/31764#discussion_r589182718
So, could you review this again freshly? And, please let me know if I missed some of your comments, (especially @viirya 's one).
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795137946
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40513/
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r591754163
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -44,12 +44,18 @@ trait CodegenInterpretedPlanTest extends PlanTest {
val codegenMode = CodegenObjectFactoryMode.CODEGEN_ONLY.toString
val interpretedMode = CodegenObjectFactoryMode.NO_CODEGEN.toString
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
- super.test(testName + " (codegen path)", testTags: _*)(testFun)(pos)
- }
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> interpretedMode) {
- super.test(testName + " (interpreted path)", testTags: _*)(testFun)(pos)
- }
+ super.test(testName + " (codegen path)", testTags: _*)(
+ withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) { testFun })(pos)
+ super.test(testName + " (interpreted path)", testTags: _*)(
+ withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> interpretedMode) { testFun })(pos)
Review comment:
Let me review all of them.
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592100903
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
##########
@@ -161,15 +161,16 @@ class ExpressionEncoderSuite extends CodegenInterpretedPlanTest with AnalysisTes
encodeDecodeTest(Seq(Seq("abc", "xyz"), Seq[String](null), null, Seq("1", null, "2")),
"seq of seq of string")
- encodeDecodeTest(Array(31, -123, 4), "array of int")
+ encodeDecodeTest(Array(31, -123, 4), "array of int", useFallback = true)
Review comment:
sounds good, let's fix these bugs one by one later.
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795833813
Since this is a long standing bug at SPARK-23596, re-ping @viirya , @HyukjinKwon , @hvanhovell , @kiszk .
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-796534680
Thank you all!
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592079803
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -44,12 +44,18 @@ trait CodegenInterpretedPlanTest extends PlanTest {
val codegenMode = CodegenObjectFactoryMode.CODEGEN_ONLY.toString
val interpretedMode = CodegenObjectFactoryMode.NO_CODEGEN.toString
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
- super.test(testName + " (codegen path)", testTags: _*)(testFun)(pos)
Review comment:
`SQLConf` is thread-local. Does `super.test` runs in another thread by the SBT test runner?
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r591753901
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/plans/PlanTest.scala
##########
@@ -44,12 +44,18 @@ trait CodegenInterpretedPlanTest extends PlanTest {
val codegenMode = CodegenObjectFactoryMode.CODEGEN_ONLY.toString
val interpretedMode = CodegenObjectFactoryMode.NO_CODEGEN.toString
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) {
- super.test(testName + " (codegen path)", testTags: _*)(testFun)(pos)
- }
- withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> interpretedMode) {
- super.test(testName + " (interpreted path)", testTags: _*)(testFun)(pos)
- }
+ super.test(testName + " (codegen path)", testTags: _*)(
+ withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> codegenMode) { testFun })(pos)
+ super.test(testName + " (interpreted path)", testTags: _*)(
+ withSQLConf(SQLConf.CODEGEN_FACTORY_MODE.key -> interpretedMode) { testFun })(pos)
Review comment:
Hmm. After fixing this correctly, I'm observing some failures like the following. It seems that I need to revisit all the existing test cases.
```
[info] - encode/decode for NestedArray: NestedArray([[I@7488b296) (codegen path) (34 milliseconds)
[info] - encode/decode for NestedArray: NestedArray([[I@7488b296) (interpreted path) *** FAILED *** (6 milliseconds)
```
----------------------------------------------------------------
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 #31766: [SPARK-34696][SQL][TESTS] Fix CodegenInterpretedPlanTest to generate correct test cases
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #31766:
URL: https://github.com/apache/spark/pull/31766#discussion_r592093684
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
##########
@@ -161,15 +161,16 @@ class ExpressionEncoderSuite extends CodegenInterpretedPlanTest with AnalysisTes
encodeDecodeTest(Seq(Seq("abc", "xyz"), Seq[String](null), null, Seq("1", null, "2")),
"seq of seq of string")
- encodeDecodeTest(Array(31, -123, 4), "array of int")
+ encodeDecodeTest(Array(31, -123, 4), "array of int", useFallback = true)
Review comment:
I also was surprised that we have several cases instead of the new two tests.
It seems to be broken some commits during the time because we didn't notice it due to this `CodegenInterpretedPlanTest` test framework bug.
Actually, I decided to focus on the test suite first and makes the recovery is the beyond of the scope of this PR.
We may had better file a new JIRA aiming the recovery of those test cases one by one if possible.
BTW, for this specific instance, the following was the error.
```
[info] - encode/decode for array of int: [I@74655d03 (interpreted path) *** FAILED *** (14 milliseconds)
[info] Exception thrown while decoding
[info] Converted: [0,1000000020,3,0,ffffff850000001f,4]
[info] Schema: value#680
[info] root
[info] -- value: array (nullable = true)
[info] |-- element: integer (containsNull = false)
[info]
[info]
[info] Encoder:
[info] class[value[0]: array<int>] (ExpressionEncoderSuite.scala:578)
[info] org.scalatest.exceptions.TestFailedException:
[info] at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
[info] at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471)
[info] at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1563)
[info] at org.scalatest.Assertions.fail(Assertions.scala:949)
[info] at org.scalatest.Assertions.fail$(Assertions.scala:945)
[info] at org.scalatest.funsuite.AnyFunSuite.fail(AnyFunSuite.scala:1563)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$encodeDecodeTest$1(ExpressionEncoderSuite.scala:578)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.verifyNotLeakingReflectionObjects(ExpressionEncoderSuite.scala:656)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$testAndVerifyNotLeakingReflectionObjects$2(ExpressionEncoderSuite.scala:669)
[info] at org.apache.spark.sql.catalyst.plans.CodegenInterpretedPlanTest.$anonfun$test$4(PlanTest.scala:50)
[info] at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf(SQLHelper.scala:54)
[info] at org.apache.spark.sql.catalyst.plans.SQLHelper.withSQLConf$(SQLHelper.scala:38)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.withSQLConf(ExpressionEncoderSuite.scala:118)
[info] at org.apache.spark.sql.catalyst.plans.CodegenInterpretedPlanTest.$anonfun$test$3(PlanTest.scala:50)
[info] at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
[info] at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
[info] at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
[info] at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
[info] at org.scalatest.Transformer.apply(Transformer.scala:22)
[info] at org.scalatest.Transformer.apply(Transformer.scala:20)
[info] at org.scalatest.funsuite.AnyFunSuiteLike$$anon$1.apply(AnyFunSuiteLike.scala:190)
[info] at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:178)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.invokeWithFixture$1(AnyFunSuiteLike.scala:188)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTest$1(AnyFunSuiteLike.scala:200)
[info] at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTest(AnyFunSuiteLike.scala:200)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTest$(AnyFunSuiteLike.scala:182)
[info] at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterEach$$super$runTest(SparkFunSuite.scala:61)
[info] at org.scalatest.BeforeAndAfterEach.runTest(BeforeAndAfterEach.scala:234)
[info] at org.scalatest.BeforeAndAfterEach.runTest$(BeforeAndAfterEach.scala:227)
[info] at org.apache.spark.SparkFunSuite.runTest(SparkFunSuite.scala:61)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTests$1(AnyFunSuiteLike.scala:233)
[info] at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:413)
[info] at scala.collection.immutable.List.foreach(List.scala:392)
[info] at org.scalatest.SuperEngine.traverseSubNodes$1(Engine.scala:401)
[info] at org.scalatest.SuperEngine.runTestsInBranch(Engine.scala:396)
[info] at org.scalatest.SuperEngine.runTestsImpl(Engine.scala:475)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTests(AnyFunSuiteLike.scala:233)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.runTests$(AnyFunSuiteLike.scala:232)
[info] at org.scalatest.funsuite.AnyFunSuite.runTests(AnyFunSuite.scala:1563)
[info] at org.scalatest.Suite.run(Suite.scala:1112)
[info] at org.scalatest.Suite.run$(Suite.scala:1094)
[info] at org.scalatest.funsuite.AnyFunSuite.org$scalatest$funsuite$AnyFunSuiteLike$$super$run(AnyFunSuite.scala:1563)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$run$1(AnyFunSuiteLike.scala:237)
[info] at org.scalatest.SuperEngine.runImpl(Engine.scala:535)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.run(AnyFunSuiteLike.scala:237)
[info] at org.scalatest.funsuite.AnyFunSuiteLike.run$(AnyFunSuiteLike.scala:236)
[info] at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterAll$$super$run(SparkFunSuite.scala:61)
[info] at org.scalatest.BeforeAndAfterAll.liftedTree1$1(BeforeAndAfterAll.scala:213)
[info] at org.scalatest.BeforeAndAfterAll.run(BeforeAndAfterAll.scala:210)
[info] at org.scalatest.BeforeAndAfterAll.run$(BeforeAndAfterAll.scala:208)
[info] at org.apache.spark.SparkFunSuite.run(SparkFunSuite.scala:61)
[info] at org.scalatest.tools.Framework.org$scalatest$tools$Framework$$runSuite(Framework.scala:318)
[info] at org.scalatest.tools.Framework$ScalaTestTask.execute(Framework.scala:513)
[info] at sbt.ForkMain$Run.lambda$runTest$1(ForkMain.java:413)
[info] at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[info] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[info] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[info] at java.lang.Thread.run(Thread.java:748)
[info] Cause: java.lang.RuntimeException: Error while decoding: java.lang.NoSuchMethodException: org.apache.spark.sql.catalyst.util.GenericArrayData.toIntArray()
[info] mapobjects(lambdavariable(MapObject, IntegerType, false, -1), assertnotnull(lambdavariable(MapObject, IntegerType, false, -1)), input[0, array<int>, true], None).toIntArray
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder$Deserializer.apply(ExpressionEncoder.scala:186)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$encodeDecodeTest$1(ExpressionEncoderSuite.scala:576)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.verifyNotLeakingReflectionObjects(ExpressionEncoderSuite.scala:656)
[info] at org.apache.spark.sql.catalyst.encoders.ExpressionEncoderSuite.$anonfun$testAndVerifyNotLeakingReflectionObjects$2(ExpressionEncoderSuite.scala:669)
```
----------------------------------------------------------------
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 #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795018697
Hi, @rednaxelafx , @cloud-fan , @viirya , @kiszk , @maropu .
Here is a summary of the progress.
1. https://github.com/apache/spark/pull/31775 (SPARK-34660 `Don't use ParVector with withExistingConf which is not thread-safe`) fixed the UT failure at `branch-3.1` last weekend.
2. https://github.com/apache/spark/pull/31785 (SPARK-33498 `Remove SQLConf.withExistingConf in CastSuite`) removes the unneeded `withExistingConf` at `master` and will be backported to branch-3.1.
So, the above two were the answer why we hit the UT failure only at `branch-3.1` while we have the patch at `master/3.1/3.0`.
Now, this PR aims to do **only refactoring PlanTest and test cases to have a clear test design**. As we discussed in the previous PR, we need this.
- https://github.com/apache/spark/pull/31764#discussion_r589170423 (@cloud-fan )
> The current test framework assumes that codegen and non-codegen should have consistent behaviors, while this codegen bug breaks the assumption. The test case fails with codegen but passes with non-codegen.
>
> +1 to add this for such test cases. One thing I'm curious about is why this test works in master...
- https://github.com/apache/spark/pull/31764#discussion_r589179603 (@maropu )
> +1 to add it, too.
I updated this PR to the master and address the two comments.
- https://github.com/apache/spark/pull/31764#discussion_r589179924
- https://github.com/apache/spark/pull/31764#discussion_r589182718
So, could you review this again freshly? And, please let me know if I missed some of your comments, (especially @viirya 's one).
----------------------------------------------------------------
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