jkasireddy commented on code in PR #3810:
URL: https://github.com/apache/iceberg-python/pull/3810#discussion_r3854462871
##########
pyiceberg/io/pyarrow.py:
##########
@@ -2349,15 +2423,44 @@ class
PyArrowStatisticsCollector(PreOrderSchemaVisitor[list[StatisticsCollector]
_schema: Schema
_properties: dict[str, str]
_default_mode: str
+ _inferred_field_ids: set[int] | None
def __init__(self, schema: Schema, properties: dict[str, str]):
from pyiceberg.table import TableProperties
self._schema = schema
self._properties = properties
- self._default_mode = self._properties.get(
- TableProperties.DEFAULT_WRITE_METRICS_MODE,
TableProperties.DEFAULT_WRITE_METRICS_MODE_DEFAULT
- )
+
+ configured_default_mode =
self._properties.get(TableProperties.DEFAULT_WRITE_METRICS_MODE)
+
+ if configured_default_mode is not None:
+ # An explicitly configured default applies to all columns.
+ self._default_mode = configured_default_mode
+ self._inferred_field_ids = None
Review Comment:
@greenlaw
Good question. I had considered that as well, but I intentionally made an
explicitly configured `write.metadata.metrics.default` bypass
`write.metadata.metrics.max-inferred-column-defaults` to match the current Java
MetricsConfig behavior.
In the Java implementation, max-inferred-column-defaults is applied only
when there is no explicitly configured default. If
`write.metadata.metrics.default=full` is set, that is treated as an explicit
user choice and the mode applies to all columns. Explicit per-column overrides
similarly take precedence over the inferred limit.
Since the property is specifically max-inferred-column-defaults, I
interpreted the limit as protecting the automatically inferred default behavior
rather than overriding an explicit global configuration.
That said, I agree that limiting an explicit full default could also be
useful for controlling manifest size. If we want PyIceberg to behave
differently here, I’m happy to adjust it, but that would diverge from the
current Java behavior. Would you prefer that we preserve Java parity for this
PR?
--
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]
