Fix TruncateTransform.satisfies_order_of crashing on different widths#3682
Fix TruncateTransform.satisfies_order_of crashing on different widths#3682SAY-5 wants to merge 1 commit into
Conversation
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
| and isinstance(other, TruncateTransform) | ||
| and isinstance(other.source_type, StringType) | ||
| ): | ||
| elif isinstance(other, TruncateTransform): |
There was a problem hiding this comment.
dropping the source_type check means an int/long/decimal transform now satisfies_order_of another purely on width. is that the intended widening, or should it stay type-gated? the java implementation only returns true within the same type family, so one type family claiming to satisfy another (string vs int) could let an invalid sort-order replacement through.
|
Good question. The For the widening concern: |
Closes #3680
Rationale for this change
TruncateTransform.satisfies_order_ofreadsself.source_type(andother.source_type) when the two transforms are not equal, butTruncateTransform.__init__only sets_widthand never sets_source_type. Comparing two truncate transforms of different widths therefore raisesAttributeError: 'TruncateTransform' object has no attribute '_source_type'instead of returning a boolean, because same-width comparisons only work by taking the earlyself == otherreturn. Two truncate transforms are ordered by width, so the comparison can be decided from the widths alone.Are these changes tested?
Yes. Added
test_truncate_satisfies_order_of_different_widthsand confirmed the existing transform tests still pass (pytest tests/test_transforms.py -k "truncate or satisfies_order"). The twotest_truncate_pyarrow_transformscases that fail locally require the optionalpyiceberg-coreextra and are unrelated to this change.Are there any user-facing changes?
No.