RussellSpitzer commented on a change in pull request #3845:
URL: https://github.com/apache/iceberg/pull/3845#discussion_r779782694
##########
File path: api/src/main/java/org/apache/iceberg/PartitionSpec.java
##########
@@ -337,16 +337,22 @@ private void checkAndAddPartitionName(String name) {
}
private void checkAndAddPartitionName(String name, Integer sourceColumnId)
{
+ checkAndAddPartitionName(name, sourceColumnId, true);
+ }
+
+ private void checkAndAddPartitionName(String name, Integer sourceColumnId,
boolean checkConflict) {
Types.NestedField schemaField = schema.findField(name);
- if (sourceColumnId != null) {
- // for identity transform case we allow conflicts between partition
and schema field name as
- // long as they are sourced from the same schema field
- Preconditions.checkArgument(schemaField == null ||
schemaField.fieldId() == sourceColumnId,
- "Cannot create identity partition sourced from different field in
schema: %s", name);
- } else {
- // for all other transforms we don't allow conflicts between partition
name and schema field name
- Preconditions.checkArgument(schemaField == null,
- "Cannot create partition from name that exists in schema: %s",
name);
+ if (checkConflict) {
Review comment:
I'm not sure the comment makes it more clear for me. I think it may be
better to just remove the comment. I think I'd still rather it was a builder
wide flag rather than an internal method with a flag but If you think it is
cleaner this way that is ok.
I have a few other possible suggestions
1. Move the Check for identity added fields to "checkCompatibility" that
way you could just do a buildUnchecked in TransformSpec to ignore the invalid
identity
2. Change transformSpec to just construct the Partition Spec without the
builder
--
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]
