Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-43936][SQL] Fix bug for toSQLId #41430

Closed
wants to merge 3 commits into from

Conversation

panbingkun
Copy link
Contributor

@panbingkun panbingkun commented Jun 2, 2023

What changes were proposed in this pull request?

The pr aims to fix issue for QueryErrorBase.toSQLId.
eg:
1.https://github.com/beliefer/spark/actions/runs/5144414857/jobs/9261862936
2.https://github.com/panbingkun/spark/actions/runs/5145364900/jobs/9262927798
3.https://github.com/panbingkun/spark/actions/runs/5143676069/jobs/9259636037

Why are the changes needed?

After SPARK-43910, __auto_generated_subquery_name from ids in errors should remove, but when the type of parts is ArrayBuffer, match will fail. causing unexpected behavior.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

  • Manually test.
  • Pass GA.

@github-actions github-actions bot added the SQL label Jun 2, 2023
@panbingkun
Copy link
Contributor Author

image

@panbingkun
Copy link
Contributor Author

panbingkun commented Jun 2, 2023

@panbingkun
Copy link
Contributor Author

panbingkun commented Jun 2, 2023

@MaxGekk

  • There seems to be a problem with this one
image
  • When the type of parts is ArrayBuffer, match will fail
image

Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 Waiting this PR be merged.

@panbingkun panbingkun changed the title [MINOR][TESTS] fix bug for natural-join [MINOR][TESTS] fix bug for toSQLId Jun 2, 2023
@panbingkun
Copy link
Contributor Author

panbingkun commented Jun 2, 2023

Manually test as follow:
image
image

@panbingkun panbingkun changed the title [MINOR][TESTS] fix bug for toSQLId [SPARK-43936][SQL] Fix bug for toSQLId Jun 2, 2023
Copy link
Contributor

@yikf yikf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, waiting CI passed.

@LuciferYang
Copy link
Contributor

local check org.apache.spark.sql.SQLQueryTestSuite, there still one case test failed, can we fixed this all in one ?Fix in other one is also ok, I think it should only failed in local test.

[info] - identifier-clause.sql *** FAILED *** (809 milliseconds)
[info]   identifier-clause.sql
[info]   Expected "...ce Name   ident
[info]   Owner  [runner]", but got "...ce Name  ident
[info]   Owner  [yangjie01]" Result did not match for query #61
[info]   DESCRIBE SCHEMA IDENTIFIER('id' || 'ent') (SQLQueryTestSuite.scala:842)
[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:1564)
[info]   at org.scalatest.Assertions.assertResult(Assertions.scala:847)
[info]   at org.scalatest.Assertions.assertResult$(Assertions.scala:842)
[info]   at org.scalatest.funsuite.AnyFunSuite.assertResult(AnyFunSuite.scala:1564)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$readGoldenFileAndCompareResults$3(SQLQueryTestSuite.scala:842)
[info]   at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:563)
[info]   at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:561)
[info]   at scala.collection.AbstractIterable.foreach(Iterable.scala:926)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.readGoldenFileAndCompareResults(SQLQueryTestSuite.scala:833)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$runQueries$10(SQLQueryTestSuite.scala:586)
[info]   at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
[info]   at org.scalatest.Assertions.withClue(Assertions.scala:1065)
[info]   at org.scalatest.Assertions.withClue$(Assertions.scala:1052)
[info]   at org.scalatest.funsuite.AnyFunSuite.withClue(AnyFunSuite.scala:1564)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.runQueries(SQLQueryTestSuite.scala:582)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$runSqlTestCase$35(SQLQueryTestSuite.scala:424)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$runSqlTestCase$35$adapted(SQLQueryTestSuite.scala:422)
[info]   at scala.collection.immutable.List.foreach(List.scala:333)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.runSqlTestCase(SQLQueryTestSuite.scala:422)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$createScalaTestCase$6(SQLQueryTestSuite.scala:329)
[info]   at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
[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:226)
[info]   at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:221)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.invokeWithFixture$1(AnyFunSuiteLike.scala:224)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTest$1(AnyFunSuiteLike.scala:236)
[info]   at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.runTest(AnyFunSuiteLike.scala:236)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.runTest$(AnyFunSuiteLike.scala:218)
[info]   at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterEach$$super$runTest(SparkFunSuite.scala:67)
[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:67)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTests$1(AnyFunSuiteLike.scala:269)
[info]   at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:413)
[info]   at scala.collection.immutable.List.foreach(List.scala:333)
[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:269)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.runTests$(AnyFunSuiteLike.scala:268)
[info]   at org.scalatest.funsuite.AnyFunSuite.runTests(AnyFunSuite.scala:1564)
[info]   at org.scalatest.Suite.run(Suite.scala:1114)
[info]   at org.scalatest.Suite.run$(Suite.scala:1096)
[info]   at org.scalatest.funsuite.AnyFunSuite.org$scalatest$funsuite$AnyFunSuiteLike$$super$run(AnyFunSuite.scala:1564)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$run$1(AnyFunSuiteLike.scala:273)
[info]   at org.scalatest.SuperEngine.runImpl(Engine.scala:535)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.run(AnyFunSuiteLike.scala:273)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.run$(AnyFunSuiteLike.scala:272)
[info]   at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterAll$$super$run(SparkFunSuite.scala:67)
[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:67)
[info]   at org.scalatest.tools.Framework.org$scalatest$tools$Framework$$runSuite(Framework.scala:321)
[info]   at org.scalatest.tools.Framework$ScalaTestTask.execute(Framework.scala:517)
[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:750)

@panbingkun
Copy link
Contributor Author

local check org.apache.spark.sql.SQLQueryTestSuite, there still one case test failed, can we fixed this all in one ?Fix in other one is also ok, I think it should only failed in local test.

[info] - identifier-clause.sql *** FAILED *** (809 milliseconds)
[info]   identifier-clause.sql
[info]   Expected "...ce Name   ident
[info]   Owner  [runner]", but got "...ce Name  ident
[info]   Owner  [yangjie01]" Result did not match for query #61
[info]   DESCRIBE SCHEMA IDENTIFIER('id' || 'ent') (SQLQueryTestSuite.scala:842)
[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:1564)
[info]   at org.scalatest.Assertions.assertResult(Assertions.scala:847)
[info]   at org.scalatest.Assertions.assertResult$(Assertions.scala:842)
[info]   at org.scalatest.funsuite.AnyFunSuite.assertResult(AnyFunSuite.scala:1564)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$readGoldenFileAndCompareResults$3(SQLQueryTestSuite.scala:842)
[info]   at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:563)
[info]   at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:561)
[info]   at scala.collection.AbstractIterable.foreach(Iterable.scala:926)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.readGoldenFileAndCompareResults(SQLQueryTestSuite.scala:833)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$runQueries$10(SQLQueryTestSuite.scala:586)
[info]   at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
[info]   at org.scalatest.Assertions.withClue(Assertions.scala:1065)
[info]   at org.scalatest.Assertions.withClue$(Assertions.scala:1052)
[info]   at org.scalatest.funsuite.AnyFunSuite.withClue(AnyFunSuite.scala:1564)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.runQueries(SQLQueryTestSuite.scala:582)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$runSqlTestCase$35(SQLQueryTestSuite.scala:424)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$runSqlTestCase$35$adapted(SQLQueryTestSuite.scala:422)
[info]   at scala.collection.immutable.List.foreach(List.scala:333)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.runSqlTestCase(SQLQueryTestSuite.scala:422)
[info]   at org.apache.spark.sql.SQLQueryTestSuite.$anonfun$createScalaTestCase$6(SQLQueryTestSuite.scala:329)
[info]   at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
[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:226)
[info]   at org.apache.spark.SparkFunSuite.withFixture(SparkFunSuite.scala:221)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.invokeWithFixture$1(AnyFunSuiteLike.scala:224)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTest$1(AnyFunSuiteLike.scala:236)
[info]   at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.runTest(AnyFunSuiteLike.scala:236)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.runTest$(AnyFunSuiteLike.scala:218)
[info]   at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterEach$$super$runTest(SparkFunSuite.scala:67)
[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:67)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$runTests$1(AnyFunSuiteLike.scala:269)
[info]   at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:413)
[info]   at scala.collection.immutable.List.foreach(List.scala:333)
[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:269)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.runTests$(AnyFunSuiteLike.scala:268)
[info]   at org.scalatest.funsuite.AnyFunSuite.runTests(AnyFunSuite.scala:1564)
[info]   at org.scalatest.Suite.run(Suite.scala:1114)
[info]   at org.scalatest.Suite.run$(Suite.scala:1096)
[info]   at org.scalatest.funsuite.AnyFunSuite.org$scalatest$funsuite$AnyFunSuiteLike$$super$run(AnyFunSuite.scala:1564)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.$anonfun$run$1(AnyFunSuiteLike.scala:273)
[info]   at org.scalatest.SuperEngine.runImpl(Engine.scala:535)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.run(AnyFunSuiteLike.scala:273)
[info]   at org.scalatest.funsuite.AnyFunSuiteLike.run$(AnyFunSuiteLike.scala:272)
[info]   at org.apache.spark.SparkFunSuite.org$scalatest$BeforeAndAfterAll$$super$run(SparkFunSuite.scala:67)
[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:67)
[info]   at org.scalatest.tools.Framework.org$scalatest$tools$Framework$$runSuite(Framework.scala:321)
[info]   at org.scalatest.tools.Framework$ScalaTestTask.execute(Framework.scala:517)
[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:750)

Let me fix the above issue in another separate PR, as the issues mentioned in the PR currently hinder the GA process.

@HyukjinKwon
Copy link
Member

@panbingkun can you just fix it here togheter?

@HyukjinKwon
Copy link
Member

Okay, the tests are almost done. Let's get this merged first.

@beliefer
Copy link
Contributor

beliefer commented Jun 2, 2023

@panbingkun Thank you for the fix. I'm waiting this one merged.

@LuciferYang
Copy link
Contributor

@LuciferYang
Copy link
Contributor

All passed, let me merge this one

@LuciferYang
Copy link
Contributor

Merged to master. Thanks @panbingkun @HyukjinKwon @beliefer @yikf @Hisoka-X

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, late LGTM. Thank you all for quick fix.

czxm pushed a commit to czxm/spark that referenced this pull request Jun 12, 2023
### What changes were proposed in this pull request?
The pr aims to fix issue for QueryErrorBase.toSQLId.
eg:
1.https://github.com/beliefer/spark/actions/runs/5144414857/jobs/9261862936
2.https://github.com/panbingkun/spark/actions/runs/5145364900/jobs/9262927798
3.https://github.com/panbingkun/spark/actions/runs/5143676069/jobs/9259636037

### Why are the changes needed?
After SPARK-43910, `__auto_generated_subquery_name` from ids in errors should remove, but when the type of `parts` is ArrayBuffer, match will fail. causing unexpected behavior.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
- Manually test.
- Pass GA.

Closes apache#41430 from panbingkun/fix_bug_natural_join.

Authored-by: panbingkun <pbk1982@gmail.com>
Signed-off-by: yangjie01 <yangjie01@baidu.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants