singhpk234 commented on code in PR #15903:
URL: https://github.com/apache/iceberg/pull/15903#discussion_r3042177999
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -593,31 +601,34 @@ private RESTTable restTableForScanPlanning(
TableIdentifier finalIdentifier,
RESTClient restClient,
Map<String, String> tableConf) {
- // Get client-side and server-side scan planning modes
- String planningModeClientConfig =
properties().get(RESTCatalogProperties.SCAN_PLANNING_MODE);
String planningModeServerConfig =
tableConf.get(RESTCatalogProperties.SCAN_PLANNING_MODE);
+ ScanPlanningMode scanPlanningModeServer =
+ planningModeServerConfig == null
+ ? null
+ :
ScanPlanningMode.valueOf(planningModeServerConfig.toUpperCase(Locale.US));
// Warn if client and server configs conflict; server config takes
precedence
- if (planningModeClientConfig != null
- && planningModeServerConfig != null
- &&
!planningModeClientConfig.equalsIgnoreCase(planningModeServerConfig)) {
+ if (scanPlanningMode != null
+ && scanPlanningModeServer != null
+ && scanPlanningMode != scanPlanningModeServer) {
LOG.warn(
"Scan planning mode mismatch for table {}: client config={}, server
config={}. "
+ "Server config will take precedence.",
finalIdentifier,
- planningModeClientConfig,
+ scanPlanningMode.name(),
planningModeServerConfig);
}
- // Determine effective mode: prefer server config if present, otherwise
use client config
- String effectiveModeConfig =
- planningModeServerConfig != null ? planningModeServerConfig :
planningModeClientConfig;
- RESTCatalogProperties.ScanPlanningMode effectiveMode =
- effectiveModeConfig != null
- ?
RESTCatalogProperties.ScanPlanningMode.fromString(effectiveModeConfig)
- : RESTCatalogProperties.SCAN_PLANNING_MODE_DEFAULT;
+ // Determine effective mode: prefer server config if present, otherwise
use client config,
+ // fall back to default if both are null
+ ScanPlanningMode effectiveMode =
+ scanPlanningModeServer != null
+ ? scanPlanningModeServer
+ : scanPlanningMode != null
+ ? scanPlanningMode
+ : RESTCatalogProperties.SCAN_PLANNING_MODE_DEFAULT;
Review Comment:
can we still maintain the 2 if ? it hard to follow a ternary within ternary
op
##########
core/src/main/java/org/apache/iceberg/rest/RESTCatalogProperties.java:
##########
@@ -69,25 +66,5 @@ public enum SnapshotMode {
public enum ScanPlanningMode {
CLIENT,
SERVER;
-
- public String modeName() {
- return name().toLowerCase(Locale.ROOT);
- }
-
- public static ScanPlanningMode fromString(String mode) {
- for (ScanPlanningMode planningMode : values()) {
- if (planningMode.modeName().equalsIgnoreCase(mode)) {
- return planningMode;
- }
- }
-
- throw new IllegalArgumentException(
- String.format(
- "Invalid scan planning mode: %s. Valid values are: %s",
- mode,
- Arrays.stream(values())
- .map(ScanPlanningMode::modeName)
- .collect(Collectors.joining(", "))));
- }
Review Comment:
we are changing the exception message by removing this, i would much rather
have the possible values defined to prompt the user to correct message.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
