linzch3 opened a new pull request, #4275: URL: https://github.com/apache/iceberg-python/pull/4275
Closes #3680 # Rationale for this change `TruncateTransform.satisfies_order_of` reads `self.source_type`, but `_source_type` is never set on `TruncateTransform`, so comparing two truncate transforms with different widths raises: ``` AttributeError: 'TruncateTransform' object has no attribute '_source_type' ``` A truncate transform doesn't know its source type, and whether a wider truncation satisfies a narrower one depends on it: - strings: ordering by `truncate[5]` implies ordering by `truncate[3]` (prefixes) - integers/decimals: it does not. `3` and `2` both truncate to `0` with width 5, but to `3` and `0` with width 3 So this PR keeps it conservative: equal transforms satisfy each other's order, and different widths return `False` instead of raising. Returning `False` only gives up an optimization, while returning `True` for integers would be wrong. Java's unbound `Truncate.satisfiesOrderOf` compares widths only. If you'd rather match Java here, I'm happy to change it. I left the `source_type` property in place to avoid an API change. This picks up the same bug as #3681, #3682 and #3854, which the stale bot closed. ## Are these changes tested? Yes. I added `test_truncate_satisfies_order_of_different_widths` in `tests/test_transforms.py`. It covers equal widths, both directions of different widths, other transform types, and the integer counterexample above. - `make test` equivalent (`pytest tests/ -m unit --ignore=tests/integration`): 4473 passed, 5 skipped. The only 4 failures are Kerberos Hive client tests that need the `kerberos` system library, which my machine doesn't have. They don't touch this code. - `prek run --files pyiceberg/transforms.py tests/test_transforms.py` (ruff, ruff format, mypy, codespell, ...): all passed. ## Are there any user-facing changes? `TruncateTransform(a).satisfies_order_of(TruncateTransform(b))` with `a != b` now returns `False` instead of raising `AttributeError`. --- I used an AI coding assistant to prepare this change. I reviewed the code and tests and ran them locally. The commit has a `Generated-by:` trailer, following the ASF generative tooling guidance. -- 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]
