rdblue commented on a change in pull request #2577:
URL: https://github.com/apache/iceberg/pull/2577#discussion_r635661142
##########
File path: spark/src/main/java/org/apache/iceberg/spark/SparkUtil.java
##########
@@ -100,4 +107,11 @@ public static void
validatePartitionTransforms(PartitionSpec spec) {
}
}
}
+
+ public static boolean isLocalityEnabledDefault(Map<String, String>
tableProperties, String fsScheme) {
+ String tableLocalityProp = PropertyUtil.propertyAsString(tableProperties,
TableProperties.LOCALITY_ENABLED,
+ TableProperties.LOCALITY_ENABLED_DEFAULT);
+ return tableLocalityProp == null ?
LOCALITY_WHITELIST_FS.contains(fsScheme) :
+ Boolean.parseBoolean(tableLocalityProp);
Review comment:
I think that locality should be enabled by default, but only relevant if
the file system is white-listed. That would make the logic here
`localityEnabled && LOCALITY_WHITELIST_FS.contains(fsScheme)`. That's a bit
simpler than using the table property to override and we don't want to check
locality for file systems that don't support 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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
