rdblue commented on code in PR #17022:
URL: https://github.com/apache/iceberg/pull/17022#discussion_r3508474922
##########
core/src/main/java/org/apache/iceberg/MetricsConfig.java:
##########
@@ -49,27 +48,39 @@
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);
+ DEFAULT_MODE,
+ 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;
- private MetricsConfig(Map<String, MetricsMode> columnModes, MetricsMode
defaultMode) {
+ private MetricsConfig(
+ Map<String, MetricsMode> columnModes,
+ MetricsMode defaultMode,
+ Map<Integer, String> idToName) {
this.columnModes = SerializableMap.copyOf(columnModes).immutableMap();
this.defaultMode = defaultMode;
+ if (idToName != null) {
Review Comment:
Right now, I don't think that we can force every code path to provide a
schema, which is how this is derived. That would be more changes and those code
paths don't need to use IDs, so I left it out. For instance, `fromProperties`
is still defined (until next release) and we can't fake a schema for that call.
We could cause it to fail, but that would defeat the purpose of deprecating it.
The approach I took here was to use `idToName` if it is present. I thought
this was a better option than causing external calls to `fromProperties` to
fail. We can tighten this up after the next release when we can guarantee all
code paths pass a schema.
--
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]
