Skip to content

TruncateTransform.satisfies_order_of raises AttributeError for different widths #3680

Description

@mattfaltyn

Apache Iceberg version

0.11.0 (latest release)

Please describe the bug 🐞

Description

Calling TruncateTransform.satisfies_order_of with two valid truncate transforms that have different widths raises an AttributeError instead of returning a boolean.

Same-width comparisons work because the method returns early when the transforms are equal.

Reproduction

frompyiceberg.transformsimportTruncateTransformTruncateTransform(5).satisfies_order_of(TruncateTransform(3))

On main at commit 48e710d20ceeeaa637d5aeae7746b787410859f8, this raises:

AttributeError: 'TruncateTransform' object has no attribute '_source_type'

The failure reproduces consistently. The same code is also present in the 0.11.1 release.

Expected behavior

The method should compare the truncate widths and return a boolean:

assertTruncateTransform(5).satisfies_order_of(TruncateTransform(3))
assertnotTruncateTransform(3).satisfies_order_of(TruncateTransform(5))

This matches the current Apache Iceberg Java implementation:

https://github.com/apache/iceberg/blob/25654ab4b29c8b5b5c20fc427da01cb70d94ed14/api/src/main/java/org/apache/iceberg/transforms/Truncate.java#L130-L141

Cause

TruncateTransform.__init__ initializes _width but not _source_type. However, satisfies_order_of still accesses the source_type property backed by _source_type:

_source_type: IcebergType=PrivateAttr()
_width: PositiveInt=PrivateAttr()
def__init__(self, width: int, **data: Any):
super().__init__(root=f"truncate[{width}]", **data)
self._width=width
defcan_transform(self, source: IcebergType) ->bool:
returnisinstance(source, (IntegerType, LongType, StringType, BinaryType, DecimalType))
defresult_type(self, source: IcebergType) ->IcebergType:
returnsource
@property
defpreserves_order(self) ->bool:
returnTrue
@property
defsource_type(self) ->IcebergType:
returnself._source_type
defproject(self, name: str, pred: BoundPredicate) ->UnboundPredicate|None:
field_type=pred.term.ref().field.field_type
ifisinstance(pred.term, BoundTransform):
return_project_transform_predicate(self, name, pred)
ifisinstance(pred, BoundUnaryPredicate):
returnpred.as_unbound(Reference(name))
elifisinstance(pred, BoundIn):
return_set_apply_transform(name, pred, self.transform(field_type))
elifisinstance(field_type, (IntegerType, LongType, DecimalType)): # type: ignore
ifisinstance(pred, BoundLiteralPredicate):
return_truncate_number(name, pred, self.transform(field_type))
elifisinstance(field_type, (BinaryType, StringType)):
ifisinstance(pred, BoundLiteralPredicate):
ifisinstance(pred, BoundNotStartsWith):
literal_width=len(pred.literal.value)
ifliteral_width<self.width:
returnpred.as_unbound(name, pred.literal.value)
elifliteral_width==self.width:
returnNotEqualTo(name, pred.literal.value)
else:
returnNone
else:
return_truncate_array(name, pred, self.transform(field_type))
defstrict_project(self, name: str, pred: BoundPredicate) ->UnboundPredicate|None:
field_type=pred.term.ref().field.field_type
ifisinstance(pred.term, BoundTransform):
return_project_transform_predicate(self, name, pred)
ifisinstance(pred, BoundUnaryPredicate):
returnpred.as_unbound(Reference(name))
ifisinstance(field_type, (IntegerType, LongType, DecimalType)):
ifisinstance(pred, BoundLiteralPredicate):
return_truncate_number_strict(name, pred, self.transform(field_type))
elifisinstance(pred, BoundNotIn):
return_set_apply_transform(name, pred, self.transform(field_type))
else:
returnNone# type: ignore
ifisinstance(pred, BoundLiteralPredicate):
ifisinstance(pred, BoundStartsWith):
literal_width=len(pred.literal.value)
ifliteral_width<self.width:
returnpred.as_unbound(name, pred.literal.value)
elifliteral_width==self.width:
returnEqualTo(name, pred.literal.value)
else:
returnNone
elifisinstance(pred, BoundNotStartsWith):
literal_width=len(pred.literal.value)
ifliteral_width<self.width:
returnpred.as_unbound(name, pred.literal.value)
elifliteral_width==self.width:
returnNotEqualTo(name, pred.literal.value)
else:
returnpred.as_unbound(name, self.transform(field_type)(pred.literal.value))
else:
# ProjectionUtil.truncateArrayStrict(name, pred, this);
return_truncate_array_strict(name, pred, self.transform(field_type))
elifisinstance(pred, BoundNotIn):
return_set_apply_transform(name, pred, self.transform(field_type))
else:
returnNone# type: ignore
@property
defwidth(self) ->int:
returnself._width
deftransform(self, source: IcebergType) ->Callable[[S|None], S|None]:
ifisinstance(source, (IntegerType, LongType)):
deftruncate_func(v: Any) ->Any:
returnv-v%self._width
elifisinstance(source, (StringType, BinaryType)):
deftruncate_func(v: Any) ->Any:
returnv[0 : min(self._width, len(v))]
elifisinstance(source, DecimalType):
deftruncate_func(v: Any) ->Any:
returntruncate_decimal(v, self._width)
else:
raiseValueError(f"Cannot truncate for type: {source}")
returnlambdav: truncate_func(v) ifvisnotNoneelseNone
defsatisfies_order_of(self, other: Transform[S, T]) ->bool:
ifself==other:
returnTrue
elif (
isinstance(self.source_type, StringType)
andisinstance(other, TruncateTransform)
andisinstance(other.source_type, StringType)
):
returnself.width>=other.width

The existing unit test only compares a transform with itself, so it returns before reaching the failing branch:

deftest_truncate_method(type_var: PrimitiveType, value: Any, expected_human_str: str, expected: Any) ->None:
truncate_transform=TruncateTransform(1) # type: ignore
assertstr(truncate_transform) ==str(eval(repr(truncate_transform)))
asserttruncate_transform.can_transform(type_var)
asserttruncate_transform.result_type(type_var) ==type_var
asserttruncate_transform.to_human_string(type_var, value) ==expected_human_str
asserttruncate_transform.transform(type_var)(value) ==expected
asserttruncate_transform.to_human_string(type_var, None) =="null"
asserttruncate_transform.width==1
asserttruncate_transform.transform(type_var)(None) isNone
asserttruncate_transform.preserves_order
asserttruncate_transform.satisfies_order_of(truncate_transform)

A focused fix could compare TruncateTransform widths directly, consistent with the Java implementation, and add regression cases for different widths.

I would be happy to contribute the fix and regression tests.

Willingness to contribute

  • I can contribute a fix for this bug independently
  • I would be willing to contribute a fix for this bug with guidance from the Iceberg community
  • I cannot contribute a fix for this bug at this time

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions