kevinjqliu commented on code in PR #1283:
URL: https://github.com/apache/iceberg-python/pull/1283#discussion_r1826131475
##########
tests/test_schema.py:
##########
@@ -1200,11 +1200,33 @@ def test_promote_float_to_double() -> None:
assert isinstance(applied.fields[0].field_type, DoubleType)
+def test_promote_long_to_int() -> None:
Review Comment:
```suggestion
def test_allow_union_long_with_int() -> None:
```
this isn't a type promotion, right? we're testing `union_by_name` now allows
type-widening
##########
tests/test_schema.py:
##########
@@ -1200,11 +1200,33 @@ def test_promote_float_to_double() -> None:
assert isinstance(applied.fields[0].field_type, DoubleType)
+def test_promote_long_to_int() -> None:
+ current_schema = Schema(NestedField(field_id=1, name="aCol",
field_type=LongType(), required=False))
+ new_schema = Schema(NestedField(field_id=1, name="aCol",
field_type=IntegerType(), required=False))
+
+ applied = UpdateSchema(transaction=None,
schema=current_schema).union_by_name(new_schema)._apply() # type: ignore
+
+ assert applied.as_struct() == current_schema.as_struct()
+ assert len(applied.fields) == 1
+ assert isinstance(applied.fields[0].field_type, LongType)
+
+
def test_detect_invalid_promotion_double_to_float() -> None:
Review Comment:
```suggestion
def test_allow_union_double_to_float() -> None:
```
or anything other than `detect_invalid`
##########
tests/test_schema.py:
##########
@@ -1200,11 +1200,33 @@ def test_promote_float_to_double() -> None:
assert isinstance(applied.fields[0].field_type, DoubleType)
+def test_promote_long_to_int() -> None:
Review Comment:
nit, I dont see a test case for int to long. Can we add that here?
Theres a corresponding test case for double to float and float to double
##########
pyiceberg/table/update/schema.py:
##########
@@ -770,7 +770,13 @@ def _update_column(self, field: NestedField,
existing_field: NestedField) -> Non
self.update_schema.make_column_optional(full_name)
if field.field_type.is_primitive and field.field_type !=
existing_field.field_type:
- self.update_schema.update_column(full_name,
field_type=field.field_type)
+ try:
+ # If the current type is wider than the new type, then
+ # we preform a noop
Review Comment:
```suggestion
# we perform a noop
```
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
