dramaticlly commented on code in PR #17022:
URL: https://github.com/apache/iceberg/pull/17022#discussion_r3508710680
##########
core/src/main/java/org/apache/iceberg/MetricsConfig.java:
##########
@@ -49,27 +49,37 @@
public final class MetricsConfig implements Serializable {
private static final Logger LOG =
LoggerFactory.getLogger(MetricsConfig.class);
- private static final Joiner DOT = Joiner.on('.');
// Disable metrics by default for wide tables to prevent excessive metadata
private static final MetricsMode DEFAULT_MODE =
MetricsModes.fromString(DEFAULT_WRITE_METRICS_MODE_DEFAULT);
- private static final MetricsConfig DEFAULT = new
MetricsConfig(ImmutableMap.of(), DEFAULT_MODE);
+ private static final MetricsConfig DEFAULT =
+ new MetricsConfig(ImmutableMap.of(), DEFAULT_MODE, ImmutableMap.of());
private static final MetricsConfig POSITION_DELETE_MODE =
new MetricsConfig(
ImmutableMap.of(
MetadataColumns.DELETE_FILE_PATH.name(),
MetricsModes.Full.get(),
MetadataColumns.DELETE_FILE_POS.name(),
MetricsModes.Full.get()),
- DEFAULT_MODE);
+ MetricsModes.None.get(),
+ ImmutableMap.of(
+ MetadataColumns.DELETE_FILE_PATH.fieldId(),
+ MetadataColumns.DELETE_FILE_PATH.name(),
+ MetadataColumns.DELETE_FILE_POS.fieldId(),
+ MetadataColumns.DELETE_FILE_POS.name()));
private final Map<String, MetricsMode> columnModes;
private final MetricsMode defaultMode;
+ private Map<Integer, String> idToName;
Review Comment:
wondering if it worth to make idToName final as well?
##########
core/src/main/java/org/apache/iceberg/MetricsConfig.java:
##########
@@ -100,32 +120,42 @@ public static MetricsConfig forTable(Table table) {
return from(table.properties(), table.schema(), table.sortOrder());
}
- /**
- * Creates a metrics config for a position delete file.
- *
- * @param table an Iceberg table
- * @deprecated This method is deprecated as of version 1.11.0 and will be
removed in 1.12.0.
- * Position deletes that include row data are no longer supported. Use
{@link
- * #forPositionDelete()} instead.
- */
- @Deprecated
- public static MetricsConfig forPositionDelete(Table table) {
- ImmutableMap.Builder<String, MetricsMode> columnModes =
ImmutableMap.builder();
+ public Iterable<Integer> metricsFieldIds() {
+ Preconditions.checkState(idToName != null, "Cannot resolve column mode by
ID: missing schema");
+ return idToName.keySet();
+ }
- columnModes.put(MetadataColumns.DELETE_FILE_PATH.name(),
MetricsModes.Full.get());
- columnModes.put(MetadataColumns.DELETE_FILE_POS.name(),
MetricsModes.Full.get());
+ public MetricsMode columnMode(int id) {
+ Preconditions.checkState(idToName != null, "Cannot resolve column mode by
ID: missing schema");
+ String name = idToName.get(id);
+ if (name != null) {
+ return columnMode(name);
+ }
- MetricsConfig tableConfig = forTable(table);
+ return defaultMode;
+ }
- MetricsMode defaultMode = tableConfig.defaultMode;
- tableConfig.columnModes.forEach(
- (columnAlias, mode) -> {
- String positionDeleteColumnAlias =
- DOT.join(MetadataColumns.DELETE_FILE_ROW_FIELD_NAME,
columnAlias);
- columnModes.put(positionDeleteColumnAlias, mode);
- });
+ public MetricsMode columnMode(String columnAlias) {
+ return columnModes.getOrDefault(columnAlias, defaultMode);
+ }
- return new MetricsConfig(columnModes.build(), defaultMode);
+ public void validateReferencedColumns(Schema schema) {
+ for (String column : columnModes.keySet()) {
+ Types.NestedField field = schema.findField(column);
+ ValidationException.check(
+ field != null,
+ "Invalid metrics config, could not find column %s from table prop %s
in schema %s",
+ column,
+ METRICS_MODE_COLUMN_CONF_PREFIX + column,
+ schema);
+
+ ValidationException.check(
+ null == idToName || column.equals(idToName.get(field.fieldId())),
Review Comment:
was thinking about the field rename and preserve the field id but I believe
it's already covered in schema update
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/SchemaUpdate.java#L517-L524
--
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]
