[SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` #43578

panbingkun · 2023-10-30T06:22:32Z

What changes were proposed in this pull request?

The pr aims to:

clean up the deprecated API usage related to MapOps.
fix method += in trait Growable is deprecated.

Why are the changes needed?

Eliminate warnings and no longer use deprecated scala APIs.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.

Manually test:

build/sbt -Phadoop-3 -Pdocker-integration-tests -Pspark-ganglia-lgpl -Pkinesis-asl -Pkubernetes -Phive-thriftserver -Pconnect -Pyarn -Phive -Phadoop-cloud -Pvolcano -Pkubernetes-integration-tests Test/package streaming-kinesis-asl-assembly/assembly connect/assembly

Was this patch authored or co-authored using generative AI tooling?

No.

…lated to `MapOps` & Fix `method += in trait Growable is deprecated`

…lated to & Fix

LuciferYang · 2023-11-01T06:21:15Z

core/src/test/scala/org/apache/spark/util/TimeStampedHashMapSuite.scala

-      assert(testMap4.size === 1)
-      assert(testMap4.get("k1").isDefined)
-      assert(testMap4("k1") === "v1")
+      testMap3.remove("k0")


Not sure if this modification aligns with the original test objective.

I found that TimeStampedHashMap is now a utility class that is no longer used by Spark, and only test cases are still using it.

I personally have the following optional suggestions:

Since it is private[spark] visibility, perhaps we can directly delete TimeStampedHashMap.

Or we can mark it as deprecated in Spark 4.0, suppress the compilation warnings with @nowarn, and then remove it in Spark 4.1.

Refactor TimeStampedHashMap into an immutable implementation to maintain the current usage.

cc @dongjoon-hyun FYI

b18d708#diff-77b12178a7036c71135074c6ddf7d659e5a69906264d5e3061087e4352e304ed introduced this data structure

After #22339, this data structure is only being used in unit tests. So, after Spark 3.0, this data structure has not been used by any production code of Spark.

+1 for (1) direct deletion.

LuciferYang · 2023-11-01T09:34:43Z

.../main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/JoinEstimation.scala

-          leftKey -> joinStat.copy(histogram = leftKeyStat.histogram),
-          rightKey -> joinStat.copy(histogram = rightKeyStat.histogram)
-        )
+        // Histograms are propagated as unchanged. During future estimation, they should be


Is it possible not to modify keyStatsAfterJoin += and the relative position of the comments?

LuciferYang · 2023-11-01T09:41:11Z

...ore/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DescribeNamespaceExec.scala

@@ -46,7 +46,8 @@ case class DescribeNamespaceExec(
    }

    if (isExtended) {
-      val properties = metadata.asScala -- CatalogV2Util.NAMESPACE_RESERVED_PROPERTIES
+      val properties = metadata.asScala.filterNot(


how about metadata.asScala.toMap --, then the subsequent properties don't need to call toMap.

Good suggestion!

LuciferYang · 2023-11-01T09:50:50Z

...ore/src/test/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalogSuite.scala

-    val toRemove =
-      CatalogV2Util.NAMESPACE_RESERVED_PROPERTIES.filter(expected.contains)
-    assert(expected -- toRemove === actual)
+    val toRemove = CatalogV2Util.NAMESPACE_RESERVED_PROPERTIES.toSet


how about change to use immutable.Map

### What changes were proposed in this pull request? The pr aims to delete `TimeStampedHashMap` and its UT. ### Why are the changes needed? During Pr #43578, we found that the class `TimeStampedHashMap` is no longer in use. Based on the suggestion, we have removed it. #43578 (comment) - First time this class `TimeStampedHashMap` be introduced: b18d708#diff-77b12178a7036c71135074c6ddf7d659e5a69906264d5e3061087e4352e304ed introduced this data structure - After #22339, this class `TimeStampedHashMap` is only being used in UT `TimeStampedHashMapSuite`. So, after Spark 3.0, this data structure has not been used by any production code of Spark. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #43633 from panbingkun/remove_TimeStampedHashMap. Authored-by: panbingkun <pbk1982@gmail.com> Signed-off-by: yangjie01 <yangjie01@baidu.com>

LuciferYang · 2023-11-02T16:39:26Z

let's rebase this one @panbingkun

panbingkun · 2023-11-02T22:19:24Z

let's rebase this one @panbingkun

Okay, thank you very much.

…ated to MapOps & Fix method += in trait Growable is deprecated

panbingkun · 2023-11-03T01:17:26Z

let's rebase this one @panbingkun

Done.

LuciferYang

+1, LGTM if test pass

panbingkun · 2023-11-03T06:49:06Z

+1, LGTM if test pass

GA has passed.
The issue of appveyor is not related to this PR.
#43631

LuciferYang · 2023-11-03T08:10:47Z

friendly ping @dongjoon-hyun do you need to take another look?

LuciferYang · 2023-11-03T15:28:19Z

Merged into master for Spark 4.0. Thanks @panbingkun and @dongjoon-hyun ~

dongjoon-hyun

+1, late LGTM. :)

[SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage re…

9cd3c43

…lated to `MapOps` & Fix `method += in trait Growable is deprecated`

github-actions bot added the CORE label Oct 30, 2023

panbingkun marked this pull request as draft October 30, 2023 06:41

[SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage re…

d4ebf7d

…lated to & Fix

github-actions bot added SQL YARN labels Oct 30, 2023

filterNot

2700b98

panbingkun marked this pull request as ready for review October 30, 2023 08:35

fix map remove bug

52da31c

LuciferYang reviewed Nov 1, 2023

View reviewed changes

panbingkun mentioned this pull request Nov 2, 2023

[SPARK-45767][CORE] Delete TimeStampedHashMap and its UT #43633

Closed

SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage rel…

a9e9af0

…ated to MapOps & Fix method += in trait Growable is deprecated

LuciferYang approved these changes Nov 3, 2023

View reviewed changes

LuciferYang closed this in ec875c5 Nov 3, 2023

dongjoon-hyun reviewed Nov 3, 2023

View reviewed changes

[SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to MapOps & Fix method += in trait Growable is deprecated #43578

[SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to MapOps & Fix method += in trait Growable is deprecated #43578

Uh oh!

Conversation

panbingkun commented Oct 30, 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?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang Nov 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuciferYang commented Nov 2, 2023

Uh oh!

panbingkun commented Nov 2, 2023

Uh oh!

panbingkun commented Nov 3, 2023

Uh oh!

LuciferYang left a comment

Choose a reason for hiding this comment

Uh oh!

panbingkun commented Nov 3, 2023

Uh oh!

LuciferYang commented Nov 3, 2023

Uh oh!

LuciferYang commented Nov 3, 2023

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` #43578

[SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` #43578

panbingkun commented Oct 30, 2023 •

edited

Loading

LuciferYang Nov 1, 2023 •

edited

Loading