You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "panbingkun (via GitHub)" <gi...@apache.org> on 2023/10/30 06:22:36 UTC
[PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
panbingkun opened a new pull request, #43578:
URL: https://github.com/apache/spark/pull/43578
### What changes were proposed in this pull request?
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1380960343
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalogSuite.scala:
##########
@@ -825,9 +825,8 @@ class V2SessionCatalogNamespaceSuite extends V2SessionCatalogBaseSuite {
expected: scala.collection.Map[String, String],
actual: scala.collection.Map[String, String]): Unit = {
// remove location and comment that are automatically added by HMS unless they are expected
- val toRemove =
- CatalogV2Util.NAMESPACE_RESERVED_PROPERTIES.filter(expected.contains)
- assert(expected -- toRemove === actual)
+ val toRemove = CatalogV2Util.NAMESPACE_RESERVED_PROPERTIES.toSet
Review Comment:
Okay.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #43578:
URL: https://github.com/apache/spark/pull/43578#issuecomment-1792021024
friendly ping @dongjoon-hyun do you need to take another look?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1378711532
##########
core/src/test/scala/org/apache/spark/util/TimeStampedHashMapSuite.scala:
##########
@@ -118,18 +118,18 @@ class TimeStampedHashMapSuite extends SparkFunSuite {
assert(testMap2.iterator.toSeq.head === (("k1", "v1")))
// +
- val testMap3 = testMap2 + (("k0", "v0"))
+ val testMap3 = testMap2 ++ Map("k0" -> "v0")
assert(testMap3.size === 2)
assert(testMap3.get("k1").isDefined)
assert(testMap3("k1") === "v1")
assert(testMap3.get("k0").isDefined)
assert(testMap3("k0") === "v0")
// -
- val testMap4 = testMap3 - "k0"
- assert(testMap4.size === 1)
- assert(testMap4.get("k1").isDefined)
- assert(testMap4("k1") === "v1")
+ testMap3.remove("k0")
Review Comment:
https://github.com/apache/spark/commit/b18d70870a33a4783c6b3b787bef9b0eec30bce0#diff-77b12178a7036c71135074c6ddf7d659e5a69906264d5e3061087e4352e304ed introduced this data structure
After https://github.com/apache/spark/pull/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.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #43578:
URL: https://github.com/apache/spark/pull/43578#issuecomment-1791642047
> let's rebase this one @panbingkun
Okay, thank you very much.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1378433085
##########
core/src/test/scala/org/apache/spark/util/TimeStampedHashMapSuite.scala:
##########
@@ -118,18 +118,18 @@ class TimeStampedHashMapSuite extends SparkFunSuite {
assert(testMap2.iterator.toSeq.head === (("k1", "v1")))
// +
- val testMap3 = testMap2 + (("k0", "v0"))
+ val testMap3 = testMap2 ++ Map("k0" -> "v0")
assert(testMap3.size === 2)
assert(testMap3.get("k1").isDefined)
assert(testMap3("k1") === "v1")
assert(testMap3.get("k0").isDefined)
assert(testMap3("k0") === "v0")
// -
- val testMap4 = testMap3 - "k0"
- assert(testMap4.size === 1)
- assert(testMap4.get("k1").isDefined)
- assert(testMap4("k1") === "v1")
+ testMap3.remove("k0")
Review Comment:
Not sure if this modification aligns with the original test objective.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang closed pull request #43578: [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated`
URL: https://github.com/apache/spark/pull/43578
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1380960587
##########
sql/core/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(
Review Comment:
Good suggestion!
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1378687555
##########
core/src/test/scala/org/apache/spark/util/TimeStampedHashMapSuite.scala:
##########
@@ -118,18 +118,18 @@ class TimeStampedHashMapSuite extends SparkFunSuite {
assert(testMap2.iterator.toSeq.head === (("k1", "v1")))
// +
- val testMap3 = testMap2 + (("k0", "v0"))
+ val testMap3 = testMap2 ++ Map("k0" -> "v0")
assert(testMap3.size === 2)
assert(testMap3.get("k1").isDefined)
assert(testMap3("k1") === "v1")
assert(testMap3.get("k0").isDefined)
assert(testMap3("k0") === "v0")
// -
- val testMap4 = testMap3 - "k0"
- assert(testMap4.size === 1)
- assert(testMap4.get("k1").isDefined)
- assert(testMap4("k1") === "v1")
+ testMap3.remove("k0")
Review Comment:
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:
1. Since it is `private[spark]` visibility, perhaps we can directly delete `TimeStampedHashMap`.
2. 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.
3. Refactor `TimeStampedHashMap` into an `immutable` implementation to maintain the current usage.
cc @dongjoon-hyun FYI
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1378591972
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/V2SessionCatalogSuite.scala:
##########
@@ -825,9 +825,8 @@ class V2SessionCatalogNamespaceSuite extends V2SessionCatalogBaseSuite {
expected: scala.collection.Map[String, String],
actual: scala.collection.Map[String, String]): Unit = {
// remove location and comment that are automatically added by HMS unless they are expected
- val toRemove =
- CatalogV2Util.NAMESPACE_RESERVED_PROPERTIES.filter(expected.contains)
- assert(expected -- toRemove === actual)
+ val toRemove = CatalogV2Util.NAMESPACE_RESERVED_PROPERTIES.toSet
Review Comment:
how about change to use `immutable.Map`
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1378711532
##########
core/src/test/scala/org/apache/spark/util/TimeStampedHashMapSuite.scala:
##########
@@ -118,18 +118,18 @@ class TimeStampedHashMapSuite extends SparkFunSuite {
assert(testMap2.iterator.toSeq.head === (("k1", "v1")))
// +
- val testMap3 = testMap2 + (("k0", "v0"))
+ val testMap3 = testMap2 ++ Map("k0" -> "v0")
assert(testMap3.size === 2)
assert(testMap3.get("k1").isDefined)
assert(testMap3("k1") === "v1")
assert(testMap3.get("k0").isDefined)
assert(testMap3("k0") === "v0")
// -
- val testMap4 = testMap3 - "k0"
- assert(testMap4.size === 1)
- assert(testMap4.get("k1").isDefined)
- assert(testMap4("k1") === "v1")
+ testMap3.remove("k0")
Review Comment:
https://github.com/apache/spark/commit/b18d70870a33a4783c6b3b787bef9b0eec30bce0#diff-77b12178a7036c71135074c6ddf7d659e5a69906264d5e3061087e4352e304ed introduced this data structure, and most of its use became ineffective after https://github.com/apache/spark/pull/126
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #43578:
URL: https://github.com/apache/spark/pull/43578#issuecomment-1791089211
let's rebase this one @panbingkun
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #43578:
URL: https://github.com/apache/spark/pull/43578#issuecomment-1792650221
Merged into master for Spark 4.0. Thanks @panbingkun and @dongjoon-hyun ~
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1378932756
##########
core/src/test/scala/org/apache/spark/util/TimeStampedHashMapSuite.scala:
##########
@@ -118,18 +118,18 @@ class TimeStampedHashMapSuite extends SparkFunSuite {
assert(testMap2.iterator.toSeq.head === (("k1", "v1")))
// +
- val testMap3 = testMap2 + (("k0", "v0"))
+ val testMap3 = testMap2 ++ Map("k0" -> "v0")
assert(testMap3.size === 2)
assert(testMap3.get("k1").isDefined)
assert(testMap3("k1") === "v1")
assert(testMap3.get("k0").isDefined)
assert(testMap3("k0") === "v0")
// -
- val testMap4 = testMap3 - "k0"
- assert(testMap4.size === 1)
- assert(testMap4.get("k1").isDefined)
- assert(testMap4("k1") === "v1")
+ testMap3.remove("k0")
Review Comment:
+1 for (1) `direct deletion`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1378576854
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/JoinEstimation.scala:
##########
@@ -206,13 +206,12 @@ case class JoinEstimation(join: Join) extends Logging {
case _ =>
computeByNdv(leftKey, rightKey, newMin, newMax)
}
- keyStatsAfterJoin += (
- // Histograms are propagated as unchanged. During future estimation, they should be
- // truncated by the updated max/min. In this way, only pointers of the histograms are
- // propagated and thus reduce memory consumption.
- leftKey -> joinStat.copy(histogram = leftKeyStat.histogram),
- rightKey -> joinStat.copy(histogram = rightKeyStat.histogram)
- )
+ // Histograms are propagated as unchanged. During future estimation, they should be
Review Comment:
Is it possible not to modify `keyStatsAfterJoin +=` and the relative position of the comments?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #43578:
URL: https://github.com/apache/spark/pull/43578#issuecomment-1791956830
> +1, LGTM if test pass
GA has passed.
The issue of `appveyor` is not related to this PR.
https://github.com/apache/spark/pull/43631
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1378711532
##########
core/src/test/scala/org/apache/spark/util/TimeStampedHashMapSuite.scala:
##########
@@ -118,18 +118,18 @@ class TimeStampedHashMapSuite extends SparkFunSuite {
assert(testMap2.iterator.toSeq.head === (("k1", "v1")))
// +
- val testMap3 = testMap2 + (("k0", "v0"))
+ val testMap3 = testMap2 ++ Map("k0" -> "v0")
assert(testMap3.size === 2)
assert(testMap3.get("k1").isDefined)
assert(testMap3("k1") === "v1")
assert(testMap3.get("k0").isDefined)
assert(testMap3("k0") === "v0")
// -
- val testMap4 = testMap3 - "k0"
- assert(testMap4.size === 1)
- assert(testMap4.get("k1").isDefined)
- assert(testMap4("k1") === "v1")
+ testMap3.remove("k0")
Review Comment:
https://github.com/apache/spark/commit/b18d70870a33a4783c6b3b787bef9b0eec30bce0#diff-77b12178a7036c71135074c6ddf7d659e5a69906264d5e3061087e4352e304ed introduced this data structure
most of its use became ineffective after https://github.com/apache/spark/pull/126
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #43578:
URL: https://github.com/apache/spark/pull/43578#discussion_r1378582880
##########
sql/core/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(
Review Comment:
how about `metadata.asScala.toMap --`, then the subsequent `properties` don't need to call `toMap`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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
Re: [PR] [SPARK-45688][SPARK-45693][CORE] Clean up the deprecated API usage related to `MapOps` & Fix `method += in trait Growable is deprecated` [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #43578:
URL: https://github.com/apache/spark/pull/43578#issuecomment-1791770240
> let's rebase this one @panbingkun
Done.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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