amogh-jahagirdar commented on code in PR #11868:
URL: https://github.com/apache/iceberg/pull/11868#discussion_r2178771819
##########
core/src/main/java/org/apache/iceberg/PartitionSpecParser.java:
##########
@@ -68,7 +68,7 @@ public static String toJson(UnboundPartitionSpec spec,
boolean pretty) {
}
public static PartitionSpec fromJson(Schema schema, JsonNode json) {
- return fromJson(json).bind(schema);
+ return fromJson(json).bindUnchecked(schema);
Review Comment:
I'm a bit concerned about changing the behavior of this API to
`bindUnchecked` since that skips over validations that still applies. If I
understand the intent,, it's because we're currently failing
[here](https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/PartitionSpec.java#L638)
I think I agree that fundamentally when parsing a spec and binding we don't
care if a source field exists or not since we'll replace it with unknown later.
However, there are some other checks in `checkCompatibility` that still seem
worth running through at this point when the sourceType is not null like making
sure that the transform and type does align, the sourcetype isn't something
like a struct etc.
It's unfortunately a bit more complicated but should we have a
`checkCompatibilityIgnoringMissing` and a `buildIgnoringMissingFields` which
just does the same checks as checkCompatibility but essentially ignoring if a
ifeld is missing, and if it is defined we still do the existing checks? Then we
largely keep the same guarantees on this function while solving the issue we
set out to solve.
--
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]
