You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "beliefer (via GitHub)" <gi...@apache.org> on 2023/12/05 10:42:00 UTC
[PR] [SPARK-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
beliefer opened a new pull request, #44183:
URL: https://github.com/apache/spark/pull/44183
### What changes were proposed in this pull request?
This PR follows up https://github.com/apache/spark/pull/43349.
This pr also does not include parts of the hive and hive-thriftserver module.
### Why are the changes needed?
Please see https://github.com/apache/spark/pull/43349.
### Does this PR introduce _any_ user-facing change?
'No'.
### How was this patch tested?
GA
### Was this patch authored or co-authored using generative AI tooling?
'No'.
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1418249978
##########
sql/hive/src/test/java/org/apache/spark/sql/hive/test/Complex.java:
##########
@@ -441,79 +441,79 @@ public void setMStringStringIsSet(boolean value) {
public void setFieldValue(_Fields field, Object value) {
switch (field) {
- case AINT:
- if (value == null) {
- unsetAint();
- } else {
- setAint((Integer)value);
- }
- break;
+ case AINT:
Review Comment:
Ya, there should be no change (including whitespace) on this file.
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1417057797
##########
sql/hive/src/test/java/org/apache/spark/sql/hive/test/Complex.java:
##########
@@ -84,22 +84,15 @@ public enum _Fields implements org.apache.thrift.TFieldIdEnum {
* Find the _Fields constant that matches fieldId, or null if its not found.
*/
public static _Fields findByThriftId(int fieldId) {
Review Comment:
OK
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1416729223
##########
common/network-common/src/main/java/org/apache/spark/network/util/DBProvider.java:
##########
@@ -38,17 +38,18 @@ public static DB initDB(
StoreVersion version,
ObjectMapper mapper) throws IOException {
if (dbFile != null) {
- switch (dbBackend) {
- case LEVELDB:
+ return switch (dbBackend) {
+ case LEVELDB -> {
org.iq80.leveldb.DB levelDB = LevelDBProvider.initLevelDB(dbFile, version, mapper);
logger.warn("The LEVELDB is deprecated. Please use ROCKSDB instead.");
- return levelDB != null ? new LevelDB(levelDB) : null;
- case ROCKSDB:
+ yield levelDB != null ? new LevelDB(levelDB) : null;
+ }
+ case ROCKSDB -> {
org.rocksdb.RocksDB rocksDB = RocksDBProvider.initRockDB(dbFile, version, mapper);
- return rocksDB != null ? new RocksDB(rocksDB) : null;
- default:
- throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend);
- }
+ yield rocksDB != null ? new RocksDB(rocksDB) : null;
+ }
+ default -> throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend);
Review Comment:
How about
```scala
return switch (dbBackend) {
case LEVELDB -> {
logger.warn("The LEVELDB is deprecated. Please use ROCKSDB instead.");
yield new LevelDB(LevelDBProvider.initLevelDB(dbFile, version, mapper));
}
case ROCKSDB -> new RocksDB(RocksDBProvider.initRockDB(dbFile, version, mapper));
};
```
`dbBackend` is an enumeration type, the default case is not necessary.
##########
launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java:
##########
@@ -143,59 +143,61 @@ static List<String> parseOptionString(String s) {
escapeNext = false;
} else if (inOpt) {
switch (c) {
- case '\\':
- if (inSingleQuote) {
- opt.appendCodePoint(c);
- } else {
- escapeNext = true;
+ case '\\' -> {
Review Comment:
For this particular case, I personally believe that `Pattern Matching for switch` does not demonstrate its advantages.
##########
sql/hive/src/test/java/org/apache/spark/sql/hive/test/Complex.java:
##########
@@ -84,22 +84,15 @@ public enum _Fields implements org.apache.thrift.TFieldIdEnum {
* Find the _Fields constant that matches fieldId, or null if its not found.
*/
public static _Fields findByThriftId(int fieldId) {
Review Comment:
this file copy from Hive 0.13, let's leaving it as it is.
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/ParquetVectorUpdaterFactory.java:
##########
@@ -181,9 +181,8 @@ public ParquetVectorUpdater getUpdater(ColumnDescriptor descriptor, DataType spa
} else if (sparkType == DataTypes.BinaryType) {
return new FixedLenByteArrayUpdater(arrayLen);
}
- break;
- default:
- break;
+ }
+ default -> {}
Review Comment:
Just to confirm, can we move
```java
throw constructConvertNotSupportedException(descriptor, sparkType);
```
from line 190 into the default branch?
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1416949962
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/ParquetVectorUpdaterFactory.java:
##########
@@ -181,9 +181,8 @@ public ParquetVectorUpdater getUpdater(ColumnDescriptor descriptor, DataType spa
} else if (sparkType == DataTypes.BinaryType) {
return new FixedLenByteArrayUpdater(arrayLen);
}
- break;
- default:
- break;
+ }
+ default -> {}
Review Comment:
I tried move the exception to the default branch. But the compiler issues error.
The reason the origin directly uses return.
The return value doesn't cover all the path of switch.
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1416935919
##########
launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java:
##########
@@ -143,59 +143,61 @@ static List<String> parseOptionString(String s) {
escapeNext = false;
} else if (inOpt) {
switch (c) {
- case '\\':
- if (inSingleQuote) {
- opt.appendCodePoint(c);
- } else {
- escapeNext = true;
+ case '\\' -> {
Review Comment:
Yes. This case can't be simplified. But it is safer than before.
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1418237100
##########
sql/hive/src/test/java/org/apache/spark/sql/hive/test/Complex.java:
##########
@@ -441,79 +441,79 @@ public void setMStringStringIsSet(boolean value) {
public void setFieldValue(_Fields field, Object value) {
switch (field) {
- case AINT:
- if (value == null) {
- unsetAint();
- } else {
- setAint((Integer)value);
- }
- break;
+ case AINT:
Review Comment:
The format of this file has changed...
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #44183:
URL: https://github.com/apache/spark/pull/44183#issuecomment-1844099331
Feel free to merge, @beliefer ~
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1418256199
##########
sql/hive/src/test/java/org/apache/spark/sql/hive/test/Complex.java:
##########
@@ -441,79 +441,79 @@ public void setMStringStringIsSet(boolean value) {
public void setFieldValue(_Fields field, Object value) {
switch (field) {
- case AINT:
- if (value == null) {
- unsetAint();
- } else {
- setAint((Integer)value);
- }
- break;
+ case AINT:
Review Comment:
Let me revert it.
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #44183:
URL: https://github.com/apache/spark/pull/44183#issuecomment-1844597695
The GA failure is unrelated.
Merged to Master.
@dongjoon-hyun @LuciferYang Thank you!
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1416970516
##########
common/network-common/src/main/java/org/apache/spark/network/util/DBProvider.java:
##########
@@ -38,17 +38,18 @@ public static DB initDB(
StoreVersion version,
ObjectMapper mapper) throws IOException {
if (dbFile != null) {
- switch (dbBackend) {
- case LEVELDB:
+ return switch (dbBackend) {
+ case LEVELDB -> {
org.iq80.leveldb.DB levelDB = LevelDBProvider.initLevelDB(dbFile, version, mapper);
logger.warn("The LEVELDB is deprecated. Please use ROCKSDB instead.");
- return levelDB != null ? new LevelDB(levelDB) : null;
- case ROCKSDB:
+ yield levelDB != null ? new LevelDB(levelDB) : null;
+ }
+ case ROCKSDB -> {
org.rocksdb.RocksDB rocksDB = RocksDBProvider.initRockDB(dbFile, version, mapper);
- return rocksDB != null ? new RocksDB(rocksDB) : null;
- default:
- throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend);
- }
+ yield rocksDB != null ? new RocksDB(rocksDB) : null;
+ }
+ default -> throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend);
Review Comment:
OK ~
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #44183:
URL: https://github.com/apache/spark/pull/44183#issuecomment-1844090656
cc @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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #44183:
URL: https://github.com/apache/spark/pull/44183#issuecomment-1841946953
@srowen @LuciferYang Please help me to review this one.
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #44183:
URL: https://github.com/apache/spark/pull/44183#issuecomment-1841945893
The GA failure is unrelated.
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1416935330
##########
common/network-common/src/main/java/org/apache/spark/network/util/DBProvider.java:
##########
@@ -38,17 +38,18 @@ public static DB initDB(
StoreVersion version,
ObjectMapper mapper) throws IOException {
if (dbFile != null) {
- switch (dbBackend) {
- case LEVELDB:
+ return switch (dbBackend) {
+ case LEVELDB -> {
org.iq80.leveldb.DB levelDB = LevelDBProvider.initLevelDB(dbFile, version, mapper);
logger.warn("The LEVELDB is deprecated. Please use ROCKSDB instead.");
- return levelDB != null ? new LevelDB(levelDB) : null;
- case ROCKSDB:
+ yield levelDB != null ? new LevelDB(levelDB) : null;
+ }
+ case ROCKSDB -> {
org.rocksdb.RocksDB rocksDB = RocksDBProvider.initRockDB(dbFile, version, mapper);
- return rocksDB != null ? new RocksDB(rocksDB) : null;
- default:
- throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend);
- }
+ yield rocksDB != null ? new RocksDB(rocksDB) : null;
+ }
+ default -> throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend);
Review Comment:
Please ignore this comments
##########
common/network-common/src/main/java/org/apache/spark/network/util/DBProvider.java:
##########
@@ -38,17 +38,18 @@ public static DB initDB(
StoreVersion version,
ObjectMapper mapper) throws IOException {
if (dbFile != null) {
- switch (dbBackend) {
- case LEVELDB:
+ return switch (dbBackend) {
+ case LEVELDB -> {
org.iq80.leveldb.DB levelDB = LevelDBProvider.initLevelDB(dbFile, version, mapper);
logger.warn("The LEVELDB is deprecated. Please use ROCKSDB instead.");
- return levelDB != null ? new LevelDB(levelDB) : null;
- case ROCKSDB:
+ yield levelDB != null ? new LevelDB(levelDB) : null;
+ }
+ case ROCKSDB -> {
org.rocksdb.RocksDB rocksDB = RocksDBProvider.initRockDB(dbFile, version, mapper);
- return rocksDB != null ? new RocksDB(rocksDB) : null;
- default:
- throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend);
- }
+ yield rocksDB != null ? new RocksDB(rocksDB) : null;
+ }
+ default -> throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend);
Review Comment:
Sorry, please ignore this 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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1416946766
##########
common/network-common/src/main/java/org/apache/spark/network/util/DBProvider.java:
##########
@@ -38,17 +38,18 @@ public static DB initDB(
StoreVersion version,
ObjectMapper mapper) throws IOException {
if (dbFile != null) {
- switch (dbBackend) {
- case LEVELDB:
+ return switch (dbBackend) {
+ case LEVELDB -> {
org.iq80.leveldb.DB levelDB = LevelDBProvider.initLevelDB(dbFile, version, mapper);
logger.warn("The LEVELDB is deprecated. Please use ROCKSDB instead.");
- return levelDB != null ? new LevelDB(levelDB) : null;
- case ROCKSDB:
+ yield levelDB != null ? new LevelDB(levelDB) : null;
+ }
+ case ROCKSDB -> {
org.rocksdb.RocksDB rocksDB = RocksDBProvider.initRockDB(dbFile, version, mapper);
- return rocksDB != null ? new RocksDB(rocksDB) : null;
- default:
- throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend);
- }
+ yield rocksDB != null ? new RocksDB(rocksDB) : null;
+ }
+ default -> throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend);
Review Comment:
We can remove the `default` here.
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1416969799
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/ParquetVectorUpdaterFactory.java:
##########
@@ -181,9 +181,8 @@ public ParquetVectorUpdater getUpdater(ColumnDescriptor descriptor, DataType spa
} else if (sparkType == DataTypes.BinaryType) {
return new FixedLenByteArrayUpdater(arrayLen);
}
- break;
- default:
- break;
+ }
+ default -> {}
Review Comment:
Got
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1418249312
##########
sql/hive/src/test/java/org/apache/spark/sql/hive/test/Complex.java:
##########
@@ -441,79 +441,79 @@ public void setMStringStringIsSet(boolean value) {
public void setFieldValue(_Fields field, Object value) {
switch (field) {
- case AINT:
- if (value == null) {
- unsetAint();
- } else {
- setAint((Integer)value);
- }
- break;
+ case AINT:
Review Comment:
Oh.. I thought this file is excluded according to your previous comment, https://github.com/apache/spark/pull/44183#discussion_r1416754836 .
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1416970217
##########
launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java:
##########
@@ -143,59 +143,61 @@ static List<String> parseOptionString(String s) {
escapeNext = false;
} else if (inOpt) {
switch (c) {
- case '\\':
- if (inSingleQuote) {
- opt.appendCodePoint(c);
- } else {
- escapeNext = true;
+ case '\\' -> {
Review Comment:
OK
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #44183:
URL: https://github.com/apache/spark/pull/44183#discussion_r1416949962
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/ParquetVectorUpdaterFactory.java:
##########
@@ -181,9 +181,8 @@ public ParquetVectorUpdater getUpdater(ColumnDescriptor descriptor, DataType spa
} else if (sparkType == DataTypes.BinaryType) {
return new FixedLenByteArrayUpdater(arrayLen);
}
- break;
- default:
- break;
+ }
+ default -> {}
Review Comment:
Good idea.
--
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-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer closed pull request #44183: [SPARK-45515][CORE][SQL][FOLLOWUP] Use enhanced switch expressions to replace the regular switch statement
URL: https://github.com/apache/spark/pull/44183
--
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