[SPARK-44507][SQL][CONNECT] Move AnalysisException to sql/api #42130

hvanhovell · 2023-07-24T16:45:55Z

What changes were proposed in this pull request?

This PR moves AnalysisException to sql/api. In order to do so we had to move the plan field out. We have introduced the ExtendedAnalysisException subclass to host this field.

Does this PR introduce any user-facing change?

Yes, AnalysisException does not have the plan field anymore. This is a breaking change. On the upside this reduces a lot of the breaking changes we had to make, for example ParseException is an AnalysisException again.

How was this patch tested?

(Updated) Existing tests.

hvanhovell · 2023-07-24T16:53:50Z

If we go with this approach we should undo some of the error work in sql/api that uses SparkException now instead of AnalysisException.

hvanhovell · 2023-07-24T16:54:05Z

cc @amaliujia @cloud-fan

zhenlineo · 2023-07-24T22:57:12Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ExtendedAnalysisException.scala

+/**
+ * Internal [[AnalysisException]] that also captures a [[LogicalPlan]].
+ */
+class ExtendedAnalysisException(


This means users would see the new error class in stacktraces? e.g.
org.apache.spark.sql.catalyst.AnalysisException -> org.apache.spark.sql.catalyst.ExtendedAnalysisException

Should we try to keep the original name by overriding toString method?

Throwable.java public String toString() { String s = getClass().getName(); String message = getLocalizedMessage(); return (message != null) ? (s + ": " + message) : s; }

I would be OK with that.

The current idea is there are a bit less place to use the plan field of the AnalysisException thus this switch cause less changes.

…ption

heyihong

👍

…ption

hvanhovell · 2023-07-27T03:25:00Z

Merging this. Tests failures are known to be problematic/flaky.

This PR moves `AnalysisException` to sql/api. In order to do so we had to move the `plan` field out. We have introduced the `ExtendedAnalysisException` subclass to host this field. Yes, `AnalysisException` does not have the `plan` field anymore. This is a breaking change. On the upside this reduces a lot of the breaking changes we had to make, for example `ParseException` is an `AnalysisException` again. (Updated) Existing tests. Closes #42130 from hvanhovell/light-analysis-exception. Authored-by: Herman van Hovell <[email protected]> Signed-off-by: Herman van Hovell <[email protected]> (cherry picked from commit 29ef893) Signed-off-by: Herman van Hovell <[email protected]>

… use AnalysisException ### What changes were proposed in this pull request? #42130 moved AnalysisException to sql/api. So we can use AnalysisException for those errors that were moved into sql/api but changed to SparkException. ### Why are the changes needed? Restore to previous behavior. ### Does this PR introduce _any_ user-facing change? No. This PR recovers the behaviors to the past which users should see AnalysisException upon many cases. ### How was this patch tested? Existing UT. Closes #42190 from amaliujia/convert_back_errors. Authored-by: Rui Wang <[email protected]> Signed-off-by: Herman van Hovell <[email protected]>

… use AnalysisException ### What changes were proposed in this pull request? #42130 moved AnalysisException to sql/api. So we can use AnalysisException for those errors that were moved into sql/api but changed to SparkException. ### Why are the changes needed? Restore to previous behavior. ### Does this PR introduce _any_ user-facing change? No. This PR recovers the behaviors to the past which users should see AnalysisException upon many cases. ### How was this patch tested? Existing UT. Closes #42190 from amaliujia/convert_back_errors. Authored-by: Rui Wang <[email protected]> Signed-off-by: Herman van Hovell <[email protected]> (cherry picked from commit 3761b7d) Signed-off-by: Herman van Hovell <[email protected]>

…te` for Java 21 ### What changes were proposed in this pull request? SPARK-44507(#42130) updated `try_arithmetic.sql.out` and `numeric.sql.out`, SPARK-44868(#42534) updated `datetime-formatting.sql.out`, but these PRs didn’t pay attention to the test health on Java 21. So this PR has regenerated the golden files `try_arithmetic.sql.out.java21`, `numeric.sql.out.java21`, and `datetime-formatting.sql.out.java21` of `SQLQueryTestSuite` so that `SQLQueryTestSuite` can be tested with Java 21. ### Why are the changes needed? Restore `SQLQueryTestSuite` to be tested with Java 21. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - Pass GitHub Actions - Manual checked: ``` java -version openjdk version "21-ea" 2023-09-19 OpenJDK Runtime Environment Zulu21+69-CA (build 21-ea+28) OpenJDK 64-Bit Server VM Zulu21+69-CA (build 21-ea+28, mixed mode, sharing) ``` ``` SPARK_GENERATE_GOLDEN_FILES=0 build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite" ``` **Before** ``` ... [info] - datetime-formatting.sql *** FAILED *** (316 milliseconds) [info] datetime-formatting.sql [info] Array("-- Automatically generated by SQLQueryTestSuite [info] ", "create temporary view v as select col from values [info] (timestamp '1582-06-01 11:33:33.123UTC+080000'), [info] (timestamp '1970-01-01 00:00:00.000Europe/Paris'), [info] (timestamp '1970-12-31 23:59:59.999Asia/Srednekolymsk'), [info] (timestamp '1996-04-01 00:33:33.123Australia/Darwin'), [info] (timestamp '2018-11-17 13:33:33.123Z'), [info] (timestamp '2020-01-01 01:33:33.123Asia/Shanghai'), [info] (timestamp '2100-01-01 01:33:33.123America/Los_Angeles') t(col) [info] ", "struct<> [info] ", " [info] [info] [info] ", "select col, date_format(col, 'G GG GGG GGGG') from v [info] ", "struct<col:timestamp,date_format(col, G GG GGG GGGG):string> [info] ", "1582-05-31 19:40:35.123 AD AD AD Anno Domini [info] 1969-12-31 15:00:00 AD AD AD Anno Domini [info] 1970-12-31 04:59:59.999 AD AD AD Anno Domini [info] 1996-03-31 07:03:33.123 AD AD AD Anno Domini [info] 2018-11-17 05:33:33.123 AD AD AD Anno Domini [info] 2019-12-31 09:33:33.123 AD AD AD Anno Domini [info] 2100-01-01 01:33:33.123 AD AD AD Anno Domini [info] [info] [info] ", "select col, date_format(col, 'y yy yyy yyyy yyyyy yyyyyy') from v [info] ", "struct<col:timestamp,date_format(col, y yy yyy yyyy yyyyy yyyyyy):string> [info] ", "1582-05-31 19:40:35.123 1582 82 1582 1582 01582 001582 [info] 1969-12-31 15:00:00 1969 69 1969 1969 01969 001969 [info] 1970-12-31 04:59:59.999 1970 70 1970 1970 01970 001970 [info] 1996-03-31 07:03:33.123 1996 96 1996 1996 01996 001996 [info] 2018-11-17 05:33:33.123 2018 18 2018 2018 02018 002018 [info] 2019-12-31 09:33:33.123 2019 19 2019 2019 02019 002019 [info] 2100-01-01 01:33:33.123 2100 00 2100 2100 02100 002100 [info] ... [info] - postgreSQL/numeric.sql *** FAILED *** (35 seconds, 848 milliseconds) [info] postgreSQL/numeric.sql [info] Expected "...rg.apache.spark.sql.[]AnalysisException [info] { [info] ...", but got "...rg.apache.spark.sql.[catalyst.Extended]AnalysisException [info] { [info] ..." Result did not match for query #544 [info] SELECT '' AS to_number_2, to_number('-34,338,492.654,878', '99G999G999D999G999') (SQLQueryTestSuite.scala:876) [info] org.scalatest.exceptions.TestFailedException: ... [info] - try_arithmetic.sql *** FAILED *** (314 milliseconds) [info] try_arithmetic.sql [info] Expected "...rg.apache.spark.sql.[]AnalysisException [info] { [info] ...", but got "...rg.apache.spark.sql.[catalyst.Extended]AnalysisException [info] { [info] ..." Result did not match for query #20 [info] SELECT try_add(interval 2 year, interval 2 second) (SQLQueryTestSuite.scala:876) [info] org.scalatest.exceptions.TestFailedException: ``` **After** ``` [info] Run completed in 9 minutes, 10 seconds. [info] Total number of tests run: 572 [info] Suites: completed 1, aborted 0 [info] Tests: succeeded 572, failed 0, canceled 0, ignored 59, pending 0 [info] All tests passed. ``` ### Was this patch authored or co-authored using generative AI tooling? No Closes #42580 from LuciferYang/SPARK-44888. Authored-by: yangjie01 <[email protected]> Signed-off-by: yangjie01 <[email protected]>

…te` for Java 21 ### What changes were proposed in this pull request? SPARK-44507(apache#42130) updated `try_arithmetic.sql.out` and `numeric.sql.out`, SPARK-44868(apache#42534) updated `datetime-formatting.sql.out`, but these PRs didn’t pay attention to the test health on Java 21. So this PR has regenerated the golden files `try_arithmetic.sql.out.java21`, `numeric.sql.out.java21`, and `datetime-formatting.sql.out.java21` of `SQLQueryTestSuite` so that `SQLQueryTestSuite` can be tested with Java 21. ### Why are the changes needed? Restore `SQLQueryTestSuite` to be tested with Java 21. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - Pass GitHub Actions - Manual checked: ``` java -version openjdk version "21-ea" 2023-09-19 OpenJDK Runtime Environment Zulu21+69-CA (build 21-ea+28) OpenJDK 64-Bit Server VM Zulu21+69-CA (build 21-ea+28, mixed mode, sharing) ``` ``` SPARK_GENERATE_GOLDEN_FILES=0 build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite" ``` **Before** ``` ... [info] - datetime-formatting.sql *** FAILED *** (316 milliseconds) [info] datetime-formatting.sql [info] Array("-- Automatically generated by SQLQueryTestSuite [info] ", "create temporary view v as select col from values [info] (timestamp '1582-06-01 11:33:33.123UTC+080000'), [info] (timestamp '1970-01-01 00:00:00.000Europe/Paris'), [info] (timestamp '1970-12-31 23:59:59.999Asia/Srednekolymsk'), [info] (timestamp '1996-04-01 00:33:33.123Australia/Darwin'), [info] (timestamp '2018-11-17 13:33:33.123Z'), [info] (timestamp '2020-01-01 01:33:33.123Asia/Shanghai'), [info] (timestamp '2100-01-01 01:33:33.123America/Los_Angeles') t(col) [info] ", "struct<> [info] ", " [info] [info] [info] ", "select col, date_format(col, 'G GG GGG GGGG') from v [info] ", "struct<col:timestamp,date_format(col, G GG GGG GGGG):string> [info] ", "1582-05-31 19:40:35.123 AD AD AD Anno Domini [info] 1969-12-31 15:00:00 AD AD AD Anno Domini [info] 1970-12-31 04:59:59.999 AD AD AD Anno Domini [info] 1996-03-31 07:03:33.123 AD AD AD Anno Domini [info] 2018-11-17 05:33:33.123 AD AD AD Anno Domini [info] 2019-12-31 09:33:33.123 AD AD AD Anno Domini [info] 2100-01-01 01:33:33.123 AD AD AD Anno Domini [info] [info] [info] ", "select col, date_format(col, 'y yy yyy yyyy yyyyy yyyyyy') from v [info] ", "struct<col:timestamp,date_format(col, y yy yyy yyyy yyyyy yyyyyy):string> [info] ", "1582-05-31 19:40:35.123 1582 82 1582 1582 01582 001582 [info] 1969-12-31 15:00:00 1969 69 1969 1969 01969 001969 [info] 1970-12-31 04:59:59.999 1970 70 1970 1970 01970 001970 [info] 1996-03-31 07:03:33.123 1996 96 1996 1996 01996 001996 [info] 2018-11-17 05:33:33.123 2018 18 2018 2018 02018 002018 [info] 2019-12-31 09:33:33.123 2019 19 2019 2019 02019 002019 [info] 2100-01-01 01:33:33.123 2100 00 2100 2100 02100 002100 [info] ... [info] - postgreSQL/numeric.sql *** FAILED *** (35 seconds, 848 milliseconds) [info] postgreSQL/numeric.sql [info] Expected "...rg.apache.spark.sql.[]AnalysisException [info] { [info] ...", but got "...rg.apache.spark.sql.[catalyst.Extended]AnalysisException [info] { [info] ..." Result did not match for query apache#544 [info] SELECT '' AS to_number_2, to_number('-34,338,492.654,878', '99G999G999D999G999') (SQLQueryTestSuite.scala:876) [info] org.scalatest.exceptions.TestFailedException: ... [info] - try_arithmetic.sql *** FAILED *** (314 milliseconds) [info] try_arithmetic.sql [info] Expected "...rg.apache.spark.sql.[]AnalysisException [info] { [info] ...", but got "...rg.apache.spark.sql.[catalyst.Extended]AnalysisException [info] { [info] ..." Result did not match for query apache#20 [info] SELECT try_add(interval 2 year, interval 2 second) (SQLQueryTestSuite.scala:876) [info] org.scalatest.exceptions.TestFailedException: ``` **After** ``` [info] Run completed in 9 minutes, 10 seconds. [info] Total number of tests run: 572 [info] Suites: completed 1, aborted 0 [info] Tests: succeeded 572, failed 0, canceled 0, ignored 59, pending 0 [info] All tests passed. ``` ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#42580 from LuciferYang/SPARK-44888. Authored-by: yangjie01 <[email protected]> Signed-off-by: yangjie01 <[email protected]>

dongjoon-hyun · 2024-05-09T20:36:36Z

Hi, @hvanhovell.

Is this JIRA ID correct? SPARK-44507 seems to be resolved as Won't Fix with no Fix Version. Could you update properly?

https://issues.apache.org/jira/browse/SPARK-44507

hvanhovell added 2 commits July 24, 2023 12:31

Split AnalysisException

cae7535

Move AnalysisException

c414f63

github-actions bot added the SQL label Jul 24, 2023

hvanhovell added 2 commits July 24, 2023 16:30

scala syntax

392bcbf

MiMa & Migration Guide

c74bc2f

github-actions bot added BUILD DOCS labels Jul 24, 2023

hvanhovell marked this pull request as ready for review July 24, 2023 21:32

More MiMa

d2d5e73

zhenlineo reviewed Jul 24, 2023

View reviewed changes

cloud-fan approved these changes Jul 25, 2023

View reviewed changes

Merge remote-tracking branch 'apache/master' into light-analysis-exce…

0c8147f

…ption

amaliujia approved these changes Jul 25, 2023

View reviewed changes

heyihong approved these changes Jul 25, 2023

View reviewed changes

hvanhovell added 12 commits July 25, 2023 09:52

Merge remote-tracking branch 'apache/master' into light-analysis-exce…

31e2658

…ption

Make it look like an AnalysisException

feb30cd

Fix tests

5012175

Make it look like an AnalysisException

a477dd2

Fix it.

e8dbf44

Merge remote-tracking branch 'apache/master' into light-analysis-exce…

1fef2ef

…ption

Fix verbatims

751fc87

Fix verbatim

d6df8ab

Fix verbatim

7ea8c63

Fix merge conflicts

24b3cc7

Fix verbatim

f3ae96a

Verbatims

1107b5c

hvanhovell closed this in 29ef893 Jul 27, 2023

amaliujia mentioned this pull request Jul 27, 2023

[SPARK-44574][SQL][CONNECT] Errors that moved into sq/api should also use AnalysisException #42190

Closed

LuciferYang mentioned this pull request Aug 21, 2023

[SPARK-44888][SQL][TESTS] Regenerate golden files of SQLQueryTestSuite for Java 21 #42580

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-44507][SQL][CONNECT] Move AnalysisException to sql/api #42130

[SPARK-44507][SQL][CONNECT] Move AnalysisException to sql/api #42130

Uh oh!

hvanhovell commented Jul 24, 2023 •

edited

Loading

Uh oh!

hvanhovell commented Jul 24, 2023

Uh oh!

hvanhovell commented Jul 24, 2023

Uh oh!

zhenlineo Jul 24, 2023

Uh oh!

hvanhovell Jul 24, 2023

Uh oh!

amaliujia Jul 25, 2023

Uh oh!

heyihong left a comment

Uh oh!

hvanhovell commented Jul 27, 2023

Uh oh!

dongjoon-hyun commented May 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[SPARK-44507][SQL][CONNECT] Move AnalysisException to sql/api #42130

[SPARK-44507][SQL][CONNECT] Move AnalysisException to sql/api #42130

Uh oh!

Conversation

hvanhovell commented Jul 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

hvanhovell commented Jul 24, 2023

Uh oh!

hvanhovell commented Jul 24, 2023

Uh oh!

zhenlineo Jul 24, 2023

Choose a reason for hiding this comment

Uh oh!

hvanhovell Jul 24, 2023

Choose a reason for hiding this comment

Uh oh!

amaliujia Jul 25, 2023

Choose a reason for hiding this comment

Uh oh!

heyihong left a comment

Choose a reason for hiding this comment

Uh oh!

hvanhovell commented Jul 27, 2023

Uh oh!

dongjoon-hyun commented May 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

hvanhovell commented Jul 24, 2023 •

edited

Loading