This is an automated email from the ASF dual-hosted git repository.
wenchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 37f8df483aa1 [SPARK-53507][CORE] Don't use case class for
BreakingChangeInfo
37f8df483aa1 is described below
commit 37f8df483aa1801f34cbaa6efec455b114336f22
Author: imarkowitz <[email protected]>
AuthorDate: Thu Oct 9 18:36:38 2025 +0800
[SPARK-53507][CORE] Don't use case class for BreakingChangeInfo
### What changes were proposed in this pull request?
Don't use a Scala case class for BreakingChangeInfo and MitigationConfig
### Why are the changes needed?
Per comment:
https://github.com/apache/spark/pull/52256#discussion_r2378561779
> [cloud-fan](https://github.com/cloud-fan) [5 days
ago](https://github.com/apache/spark/pull/52256#discussion_r2378561779)
> I just realize that we expose it as a public API via
SparkThrowable.getBreakingChangeInfo. We shouldn't expose a case class as
public API as it has a wide API surface, including the companion object.
> We should follow SparkThrowable and define it in Java.
### Does this PR introduce _any_ user-facing change?
No -- interface is almost the same
### How was this patch tested?
Updated unit tests
### Was this patch authored or co-authored using generative AI tooling?
Used Github co-pilot, mainly for the `equals` and `hashCode` functions
Closes #52484 from imarkowitz/ian/breaking-changes-case-class.
Lead-authored-by: imarkowitz <[email protected]>
Co-authored-by: Wenchen Fan <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
---
.../org/apache/spark/ErrorClassesJSONReader.scala | 42 ++++++++++++++++++----
.../org/apache/spark/SparkThrowableSuite.scala | 10 +++---
.../service/FetchErrorDetailsHandlerSuite.scala | 6 ++--
3 files changed, 44 insertions(+), 14 deletions(-)
diff --git
a/common/utils/src/main/scala/org/apache/spark/ErrorClassesJSONReader.scala
b/common/utils/src/main/scala/org/apache/spark/ErrorClassesJSONReader.scala
index 8a9adf7f87ad..b2415920c9bf 100644
--- a/common/utils/src/main/scala/org/apache/spark/ErrorClassesJSONReader.scala
+++ b/common/utils/src/main/scala/org/apache/spark/ErrorClassesJSONReader.scala
@@ -203,18 +203,48 @@ private case class ErrorSubInfo(
* If false, the spark job should be retried by setting
the
* mitigationConfig.
*/
-case class BreakingChangeInfo(
- migrationMessage: Seq[String],
- mitigationConfig: Option[MitigationConfig] = None,
- needsAudit: Boolean = true
-)
+class BreakingChangeInfo(
+ val migrationMessage: Seq[String],
+ val mitigationConfig: Option[MitigationConfig] = None,
+ val needsAudit: Boolean = true) {
+ override def equals(other: Any): Boolean = other match {
+ case that: BreakingChangeInfo =>
+ migrationMessage == that.migrationMessage &&
+ mitigationConfig == that.mitigationConfig &&
+ needsAudit == that.needsAudit
+ case _ => false
+ }
+
+ override def hashCode(): Int = {
+ val prime = 31
+ var result = 1
+ result = prime * result + migrationMessage.hashCode()
+ result = prime * result + mitigationConfig.hashCode()
+ result = prime * result + needsAudit.hashCode()
+ result
+ }
+}
/**
* A spark config flag that can be used to mitigate a breaking change.
* @param key The spark config key.
* @param value The spark config value that mitigates the breaking change.
*/
-case class MitigationConfig(key: String, value: String)
+class MitigationConfig(val key: String, val value: String) {
+ override def equals(other: Any): Boolean = other match {
+ case that: MitigationConfig =>
+ key == that.key && value == that.value
+ case _ => false
+ }
+
+ override def hashCode(): Int = {
+ val prime = 31
+ var result = 1
+ result = prime * result + key.hashCode()
+ result = prime * result + value.hashCode()
+ result
+ }
+}
/**
* Information associated with an error state / SQLSTATE.
diff --git a/core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala
b/core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala
index 61ffe9b4bd93..897ff4f891e9 100644
--- a/core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala
+++ b/core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala
@@ -570,10 +570,10 @@ class SparkThrowableSuite extends SparkFunSuite {
val breakingChangeInfo = reader.getBreakingChangeInfo("TEST_ERROR")
assert(
breakingChangeInfo.contains(
- BreakingChangeInfo(
+ new BreakingChangeInfo(
Seq("Migration message with <param2>."),
- Some(MitigationConfig("config.key1", "config.value1")),
- needsAudit = false)))
+ Some(new MitigationConfig("config.key1", "config.value1")),
+ false)))
val errorMessage2 =
reader.getErrorMessage("TEST_ERROR_WITH_SUBCLASS.SUBCLASS",
error2Params)
assert(
@@ -582,9 +582,9 @@ class SparkThrowableSuite extends SparkFunSuite {
val breakingChangeInfo2 =
reader.getBreakingChangeInfo("TEST_ERROR_WITH_SUBCLASS.SUBCLASS")
assert(
breakingChangeInfo2.contains(
- BreakingChangeInfo(
+ new BreakingChangeInfo(
Seq("Subclass migration message with <param3>."),
- Some(MitigationConfig("config.key2", "config.value2")))))
+ Some(new MitigationConfig("config.key2", "config.value2")))))
}
}
diff --git
a/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
b/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
index f389b753ad01..569ee9737c69 100644
---
a/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
+++
b/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
@@ -291,9 +291,9 @@ class FetchErrorDetailsHandlerSuite extends
SharedSparkSession with ResourceHelp
val migrationMessages =
Seq("Please update your code to use new API", "See documentation for
details")
val mitigationConfig =
- Some(MitigationConfig("spark.sql.legacy.behavior.enabled", "true"))
+ Some(new MitigationConfig("spark.sql.legacy.behavior.enabled", "true"))
val breakingChangeInfo =
- BreakingChangeInfo(migrationMessages, mitigationConfig, needsAudit =
false)
+ new BreakingChangeInfo(migrationMessages, mitigationConfig, false)
val testError = new TestSparkThrowableWithBreakingChange(
"TEST_BREAKING_CHANGE_ERROR",
@@ -342,7 +342,7 @@ class FetchErrorDetailsHandlerSuite extends
SharedSparkSession with ResourceHelp
test(
"throwableToFetchErrorDetailsResponse with breaking change info without
mitigation config") {
val migrationMessages = Seq("Migration message only")
- val breakingChangeInfo = BreakingChangeInfo(migrationMessages, None,
needsAudit = true)
+ val breakingChangeInfo = new BreakingChangeInfo(migrationMessages, None,
true)
val testError = new TestSparkThrowableWithBreakingChange(
"TEST_BREAKING_CHANGE_NO_MITIGATION",
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
