ewoodbury commented on code in PR #3431:
URL: https://github.com/apache/iceberg-rust/pull/3431#discussion_r4306167078
##########
crates/iceberg/src/spec/partition.rs:
##########
@@ -709,14 +715,15 @@ impl PartitionSpecBuilder {
/// Ensure that the partition name is unique among columns in the schema.
/// Duplicate names are allowed if:
/// 1. The column is sourced from the column with the same name.
- /// 2. AND the transformation is identity
+ /// 2. AND the transformation is identity or void. A void field named
after its source is
+ /// what removing an identity field from a format version 1 spec leaves
behind.
fn check_name_does_not_collide_with_schema(
field: &UnboundPartitionField,
schema: &Schema,
) -> Result<()> {
match schema.field_by_name(field.name.as_str()) {
Some(schema_collision) => {
- if field.transform == Transform::Identity {
+ if matches!(field.transform, Transform::Identity |
Transform::Void) {
Review Comment:
Small one: this branch now covers void too, but the error just below still
says "Cannot create identity partition sourced from different field". So does
the copy in `validate_partition_field_names` (`table_metadata_builder.rs:769`).
I'd prefer both say "identity or void partition", which matches what was
suggested on #3215. A one-line "keep in sync" note between the two `Identity |
Void` checks might also help.
--
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]
