rdblue commented on code in PR #17960:
URL: https://github.com/apache/iceberg/pull/17960#discussion_r4108663485
##########
core/src/main/java/org/apache/iceberg/MetricsConfig.java:
##########
@@ -255,9 +260,36 @@ public Set<Integer> map(
* @return metrics configuration
*/
public static MetricsConfig from(Map<String, String> props, Schema schema,
SortOrder order) {
- int maxInferredDefaultColumns = maxInferredColumnDefaults(props);
Map<Integer, String> idToName = Maps.newHashMap();
Map<String, MetricsMode> columnModes = Maps.newHashMap();
+ MetricsMode defaultMode = defaultModeFor(props, schema, order,
columnModes, idToName);
+ return new MetricsConfig(columnModes, defaultMode, idToName);
+ }
+
+ private static MetricsConfig from(
+ Map<String, String> props,
+ Schema schema,
+ SortOrder order,
+ PartitionSpec spec,
+ int formatVersion) {
+ Map<Integer, String> idToName = Maps.newHashMap();
+ Map<String, MetricsMode> columnModes = Maps.newHashMap();
+ MetricsMode defaultMode = defaultModeFor(props, schema, order,
columnModes, idToName);
+
+ if (formatVersion >= 4) {
+ configureFullMetricsForPartitionSourceColumns(spec, schema, columnModes,
idToName);
Review Comment:
I think that we want to do this for all format versions. When you upgrade to
v4, it's better to have tighter metrics than what we can infer from the
partition values. And the number of columns affected by this would be very
small. I think we can just make this change without worrying about anyone
having issues.
--
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]
