rambleraptor commented on code in PR #14266:
URL: https://github.com/apache/iceberg/pull/14266#discussion_r2543401611
##########
api/src/main/java/org/apache/iceberg/types/CheckCompatibility.java:
##########
@@ -38,7 +38,21 @@ public class CheckCompatibility extends
TypeUtil.CustomOrderSchemaVisitor<List<S
* @return a list of error details, or an empty list if there are no
compatibility problems
*/
public static List<String> writeCompatibilityErrors(Schema readSchema,
Schema writeSchema) {
- return writeCompatibilityErrors(readSchema, writeSchema, true);
+ return writeCompatibilityErrors(readSchema, writeSchema, true, 2);
Review Comment:
There's been places I've seen that have a DEFAULT_FORMAT_VERSION. I've
replaced these references to use a DEFAULT_FORMAT_VERSION to consolidate.
##########
api/src/main/java/org/apache/iceberg/types/CheckCompatibility.java:
##########
@@ -84,7 +121,13 @@ public static List<String> typeCompatibilityErrors(
* @return a list of error details, or an empty list if there are no
compatibility problems
*/
public static List<String> typeCompatibilityErrors(Schema readSchema, Schema
writeSchema) {
Review Comment:
Same comment as above. The pattern I've seen (to not break APIs) is to have
a DEFAULT_FORMAT_VERSION.
##########
core/src/main/java/org/apache/iceberg/SchemaUpdate.java:
##########
@@ -65,27 +65,48 @@ class SchemaUpdate implements UpdateSchema {
private boolean allowIncompatibleChanges = false;
private Set<String> identifierFieldNames;
private boolean caseSensitive = true;
+ private final int formatVersion;
SchemaUpdate(TableOperations ops) {
this(ops, ops.current());
}
+ /** For testing only. */
+ SchemaUpdate(TableMetadata base) {
+ this(null, base, base.schema(), base.lastColumnId(), base.formatVersion());
+ }
+
/** For testing only. */
SchemaUpdate(Schema schema, int lastColumnId) {
- this(null, null, schema, lastColumnId);
+ this(null, null, schema, lastColumnId,
TableProperties.DEFAULT_FORMAT_VERSION);
Review Comment:
This is a testing only API, so I'm assuming it's fine.
--
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]
