kevinjqliu commented on code in PR #2305:
URL: https://github.com/apache/iceberg-python/pull/2305#discussion_r2284124849


##########
pyiceberg/table/update/schema.py:
##########
@@ -658,6 +658,14 @@ def _apply(self) -> Schema:
 
         # Check the field-ids
         new_schema = Schema(*struct.fields)
+        if self._transaction is not None:
+            from pyiceberg.partitioning import validate_partition_name
+
+            for spec in self._transaction.table_metadata.partition_specs:
+                for partition_field in spec.fields:
+                    validate_partition_name(
+                        partition_field.name, partition_field.transform, 
partition_field.source_id, new_schema
+                    )

Review Comment:
   looks like everywhere else in the codebase we include transaction in 
UpdateSchema.
   
   Maybe we can update the tests like this
   ```
   def test_add_top_level_primitives(primitive_fields: List[NestedField], 
table_v2: Table) -> None:
       for primitive_field in primitive_fields:
           new_schema = Schema(primitive_field)
           applied = UpdateSchema(transaction=Transaction(table_v2), 
schema=Schema()).union_by_name(new_schema)._apply()  # type: ignore
           assert applied == new_schema
   ```



##########
tests/integration/test_partition_evolution.py:
##########
@@ -63,13 +63,22 @@ def _table_v2(catalog: Catalog) -> Table:
     return _create_table_with_schema(catalog, schema_with_timestamp, "2")
 
 
-def _create_table_with_schema(catalog: Catalog, schema: Schema, 
format_version: str) -> Table:
+def _create_table_with_schema(
+    catalog: Catalog, schema: Schema, format_version: str, partition_spec: 
PartitionSpec = UNPARTITIONED_PARTITION_SPEC
+) -> Table:
     tbl_name = "default.test_schema_evolution"
     try:
         catalog.drop_table(tbl_name)
     except NoSuchTableError:
         pass
-    return catalog.create_table(identifier=tbl_name, schema=schema, 
properties={"format-version": format_version})
+
+    if partition_spec:
+        return catalog.create_table(
+            identifier=tbl_name, schema=schema, partition_spec=partition_spec, 
properties={"format-version": format_version}
+        )
+    return catalog.create_table(
+        identifier=tbl_name, schema=schema, partition_spec=partition_spec, 
properties={"format-version": format_version}
+    )

Review Comment:
   ```suggestion
       return catalog.create_table(
           identifier=tbl_name, schema=schema, partition_spec=partition_spec, 
properties={"format-version": format_version}
       )
   ```



-- 
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]

Reply via email to