You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "rdblue (via GitHub)" <gi...@apache.org> on 2024/10/30 17:21:28 UTC
[PR] API: Add compatibility checks for Schemas with default values [iceberg]
rdblue opened a new pull request, #11434:
URL: https://github.com/apache/iceberg/pull/11434
This updates `Schema.checkCompatibility`:
* Adds a check that `initial-default` is not set for v1 and v2 tables, which would break forward compatibility
* Updates the method to return a list of compatibility problems rather than one at a time
* Adds tests for `checkCompatibility` in `TestSchema`
I think that this should get into 1.7.0 to avoid accidentally leaking initial defaults in v1 and v2 tables.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#issuecomment-2447996818
Tests failing are because Metadata.json also has validations for this and this check is failing first
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#issuecomment-2448510107
Thanks everyone - @rdblue (for original fix), @Fokko , @anuragmantri , @kevinjqliu , @singhpk234 , @amogh-jahagirdar For reviews!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#discussion_r1823318178
##########
api/src/main/java/org/apache/iceberg/Schema.java:
##########
@@ -586,16 +587,37 @@ private List<NestedField> reassignIds(List<NestedField> columns, TypeUtil.GetID
* @param formatVersion table format version
*/
public static void checkCompatibility(Schema schema, int formatVersion) {
- // check the type in each field
+ // accumulate errors as a treemap to keep them in a reasonable order
+ Map<Integer, String> problems = Maps.newTreeMap();
Review Comment:
Or any type parameter I think ... I feel like this is a 1337code question
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#discussion_r1823309114
##########
api/src/main/java/org/apache/iceberg/Schema.java:
##########
@@ -586,16 +587,37 @@ private List<NestedField> reassignIds(List<NestedField> columns, TypeUtil.GetID
* @param formatVersion table format version
*/
public static void checkCompatibility(Schema schema, int formatVersion) {
- // check the type in each field
+ // accumulate errors as a treemap to keep them in a reasonable order
+ Map<Integer, String> problems = Maps.newTreeMap();
Review Comment:
[optional] if we are going for another revision as fieldId() is `int` :
```suggestion
Map<int, String> problems = Maps.newTreeMap();
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#discussion_r1823317138
##########
api/src/main/java/org/apache/iceberg/Schema.java:
##########
@@ -586,16 +587,37 @@ private List<NestedField> reassignIds(List<NestedField> columns, TypeUtil.GetID
* @param formatVersion table format version
*/
public static void checkCompatibility(Schema schema, int formatVersion) {
- // check the type in each field
+ // accumulate errors as a treemap to keep them in a reasonable order
+ Map<Integer, String> problems = Maps.newTreeMap();
Review Comment:
Primitives are not allowed as Map Keys :(
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#discussion_r1823174316
##########
api/src/main/java/org/apache/iceberg/Schema.java:
##########
@@ -586,16 +588,37 @@ private List<NestedField> reassignIds(List<NestedField> columns, TypeUtil.GetID
* @param formatVersion table format version
*/
public static void checkCompatibility(Schema schema, int formatVersion) {
- // check the type in each field
+ // accumulate errors as a treemap to keep them in a reasonable order
+ TreeMap<Integer, String> problems = Maps.newTreeMap();
+
+ // check each field's type and defaults
for (NestedField field : schema.lazyIdToField().values()) {
Integer minFormatVersion = MIN_FORMAT_VERSIONS.get(field.type().typeId());
- Preconditions.checkState(
- minFormatVersion == null || formatVersion >= minFormatVersion,
- "Invalid type in v%s schema: %s %s is not supported until v%s",
- formatVersion,
- schema.findColumnName(field.fieldId()),
- field.type(),
- minFormatVersion);
+ if (minFormatVersion != null && formatVersion < minFormatVersion) {
+ problems.put(
+ field.fieldId(),
+ String.format(
+ "Invalid type for %s: %s is not supported until v%s",
Review Comment:
Existing checks in TableMetadata look like
```
"Invalid type in v%s schema: struct.ts_nanos timestamptz_ns is not supported until v3"
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#issuecomment-2448072176
Updated. Thanks, @RussellSpitzer!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#discussion_r1823307130
##########
api/src/main/java/org/apache/iceberg/Schema.java:
##########
@@ -54,6 +54,7 @@ public class Schema implements Serializable {
private static final Joiner NEWLINE = Joiner.on('\n');
private static final String ALL_COLUMNS = "*";
private static final int DEFAULT_SCHEMA_ID = 0;
+ private static final Integer DEFAULT_VALUES_MIN_FORMAT_VERSION = 3;
Review Comment:
```suggestion
private static final int DEFAULT_VALUES_MIN_FORMAT_VERSION = 3;
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#discussion_r1823152403
##########
api/src/test/java/org/apache/iceberg/TestSchema.java:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import org.apache.iceberg.types.Types;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class TestSchema {
+ private static final Schema TS_NANO_CASES =
+ new Schema(
+ Types.NestedField.required(1, "id", Types.LongType.get()),
+ Types.NestedField.optional(2, "ts", Types.TimestampNanoType.withZone()),
+ Types.NestedField.optional(
+ 3, "arr", Types.ListType.ofRequired(4, Types.TimestampNanoType.withoutZone())),
+ Types.NestedField.required(
+ 5,
+ "struct",
+ Types.StructType.of(
+ Types.NestedField.optional(6, "inner_ts", Types.TimestampNanoType.withZone()),
+ Types.NestedField.required(7, "data", Types.StringType.get()))),
+ Types.NestedField.optional(
+ 8,
+ "struct_arr",
+ Types.StructType.of(
+ Types.NestedField.optional(9, "ts", Types.TimestampNanoType.withoutZone()))));
+
+ private static final Schema INITIAL_DEFAULT_SCHEMA =
+ new Schema(
+ Types.NestedField.required(1, "id", Types.LongType.get()),
+ Types.NestedField.required("has_default")
+ .withId(2)
+ .ofType(Types.StringType.get())
+ .withInitialDefault("--")
+ .withWriteDefault("--")
+ .build());
+
+ private static final Schema WRITE_DEFAULT_SCHEMA =
+ new Schema(
+ Types.NestedField.required(1, "id", Types.LongType.get()),
+ Types.NestedField.required("has_default")
+ .withId(2)
+ .ofType(Types.StringType.get())
+ .withWriteDefault("--")
+ .build());
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2})
+ public void testUnsupportedTimestampNano(int formatVersion) {
+ Assertions.assertThatThrownBy(() -> Schema.checkCompatibility(TS_NANO_CASES, formatVersion))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage(
+ "Invalid schema for v%s:\n"
+ + "- Invalid type for ts: timestamptz_ns is not supported until v3\n"
+ + "- Invalid type for arr.element: timestamp_ns is not supported until v3\n"
+ + "- Invalid type for struct.inner_ts: timestamptz_ns is not supported until v3\n"
+ + "- Invalid type for struct_arr.ts: timestamp_ns is not supported until v3",
+ formatVersion);
+ }
+
+ @Test
+ public void testSupportedTimestampNano() {
+ Assertions.assertThatCode(() -> Schema.checkCompatibility(TS_NANO_CASES, 3))
+ .doesNotThrowAnyException();
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2})
+ public void testUnsupportedInitialDefault(int formatVersion) {
+ Assertions.assertThatThrownBy(
+ () -> Schema.checkCompatibility(INITIAL_DEFAULT_SCHEMA, formatVersion))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage(
+ "Invalid schema for v%s:\n"
+ + "- Invalid initial default for has_default: "
+ + "non-null default (--) is not supported until v3",
+ formatVersion);
+ }
+
+ @Test
+ public void testSupportedInitialDefault() {
+ Assertions.assertThatCode(() -> Schema.checkCompatibility(INITIAL_DEFAULT_SCHEMA, 3))
+ .doesNotThrowAnyException();
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2, 3})
+ public void testSupportedInitialDefault(int formatVersion) {
Review Comment:
testSupportedWriterDefault?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#discussion_r1823152850
##########
api/src/test/java/org/apache/iceberg/TestSchema.java:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import org.apache.iceberg.types.Types;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class TestSchema {
+ private static final Schema TS_NANO_CASES =
+ new Schema(
+ Types.NestedField.required(1, "id", Types.LongType.get()),
+ Types.NestedField.optional(2, "ts", Types.TimestampNanoType.withZone()),
+ Types.NestedField.optional(
+ 3, "arr", Types.ListType.ofRequired(4, Types.TimestampNanoType.withoutZone())),
+ Types.NestedField.required(
+ 5,
+ "struct",
+ Types.StructType.of(
+ Types.NestedField.optional(6, "inner_ts", Types.TimestampNanoType.withZone()),
+ Types.NestedField.required(7, "data", Types.StringType.get()))),
+ Types.NestedField.optional(
+ 8,
+ "struct_arr",
+ Types.StructType.of(
+ Types.NestedField.optional(9, "ts", Types.TimestampNanoType.withoutZone()))));
+
+ private static final Schema INITIAL_DEFAULT_SCHEMA =
+ new Schema(
+ Types.NestedField.required(1, "id", Types.LongType.get()),
+ Types.NestedField.required("has_default")
+ .withId(2)
+ .ofType(Types.StringType.get())
+ .withInitialDefault("--")
+ .withWriteDefault("--")
+ .build());
+
+ private static final Schema WRITE_DEFAULT_SCHEMA =
+ new Schema(
+ Types.NestedField.required(1, "id", Types.LongType.get()),
+ Types.NestedField.required("has_default")
+ .withId(2)
+ .ofType(Types.StringType.get())
+ .withWriteDefault("--")
+ .build());
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2})
+ public void testUnsupportedTimestampNano(int formatVersion) {
+ Assertions.assertThatThrownBy(() -> Schema.checkCompatibility(TS_NANO_CASES, formatVersion))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage(
+ "Invalid schema for v%s:\n"
+ + "- Invalid type for ts: timestamptz_ns is not supported until v3\n"
+ + "- Invalid type for arr.element: timestamp_ns is not supported until v3\n"
+ + "- Invalid type for struct.inner_ts: timestamptz_ns is not supported until v3\n"
+ + "- Invalid type for struct_arr.ts: timestamp_ns is not supported until v3",
+ formatVersion);
+ }
+
+ @Test
+ public void testSupportedTimestampNano() {
+ Assertions.assertThatCode(() -> Schema.checkCompatibility(TS_NANO_CASES, 3))
+ .doesNotThrowAnyException();
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2})
+ public void testUnsupportedInitialDefault(int formatVersion) {
+ Assertions.assertThatThrownBy(
+ () -> Schema.checkCompatibility(INITIAL_DEFAULT_SCHEMA, formatVersion))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage(
+ "Invalid schema for v%s:\n"
+ + "- Invalid initial default for has_default: "
+ + "non-null default (--) is not supported until v3",
+ formatVersion);
+ }
+
+ @Test
+ public void testSupportedInitialDefault() {
+ Assertions.assertThatCode(() -> Schema.checkCompatibility(INITIAL_DEFAULT_SCHEMA, 3))
+ .doesNotThrowAnyException();
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2, 3})
+ public void testSupportedInitialDefault(int formatVersion) {
Review Comment:
```suggestion
public void testSupportedWriterDefault(int formatVersion) {
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #11434:
URL: https://github.com/apache/iceberg/pull/11434#discussion_r1823154660
##########
api/src/test/java/org/apache/iceberg/TestSchema.java:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg;
+
+import org.apache.iceberg.types.Types;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class TestSchema {
+ private static final Schema TS_NANO_CASES =
+ new Schema(
+ Types.NestedField.required(1, "id", Types.LongType.get()),
+ Types.NestedField.optional(2, "ts", Types.TimestampNanoType.withZone()),
+ Types.NestedField.optional(
+ 3, "arr", Types.ListType.ofRequired(4, Types.TimestampNanoType.withoutZone())),
+ Types.NestedField.required(
+ 5,
+ "struct",
+ Types.StructType.of(
+ Types.NestedField.optional(6, "inner_ts", Types.TimestampNanoType.withZone()),
+ Types.NestedField.required(7, "data", Types.StringType.get()))),
+ Types.NestedField.optional(
+ 8,
+ "struct_arr",
+ Types.StructType.of(
+ Types.NestedField.optional(9, "ts", Types.TimestampNanoType.withoutZone()))));
+
+ private static final Schema INITIAL_DEFAULT_SCHEMA =
+ new Schema(
+ Types.NestedField.required(1, "id", Types.LongType.get()),
+ Types.NestedField.required("has_default")
+ .withId(2)
+ .ofType(Types.StringType.get())
+ .withInitialDefault("--")
+ .withWriteDefault("--")
+ .build());
+
+ private static final Schema WRITE_DEFAULT_SCHEMA =
+ new Schema(
+ Types.NestedField.required(1, "id", Types.LongType.get()),
+ Types.NestedField.required("has_default")
+ .withId(2)
+ .ofType(Types.StringType.get())
+ .withWriteDefault("--")
+ .build());
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2})
+ public void testUnsupportedTimestampNano(int formatVersion) {
+ Assertions.assertThatThrownBy(() -> Schema.checkCompatibility(TS_NANO_CASES, formatVersion))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage(
+ "Invalid schema for v%s:\n"
+ + "- Invalid type for ts: timestamptz_ns is not supported until v3\n"
+ + "- Invalid type for arr.element: timestamp_ns is not supported until v3\n"
+ + "- Invalid type for struct.inner_ts: timestamptz_ns is not supported until v3\n"
+ + "- Invalid type for struct_arr.ts: timestamp_ns is not supported until v3",
+ formatVersion);
+ }
+
+ @Test
+ public void testSupportedTimestampNano() {
+ Assertions.assertThatCode(() -> Schema.checkCompatibility(TS_NANO_CASES, 3))
+ .doesNotThrowAnyException();
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2})
+ public void testUnsupportedInitialDefault(int formatVersion) {
+ Assertions.assertThatThrownBy(
+ () -> Schema.checkCompatibility(INITIAL_DEFAULT_SCHEMA, formatVersion))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage(
+ "Invalid schema for v%s:\n"
+ + "- Invalid initial default for has_default: "
+ + "non-null default (--) is not supported until v3",
+ formatVersion);
+ }
+
+ @Test
+ public void testSupportedInitialDefault() {
+ Assertions.assertThatCode(() -> Schema.checkCompatibility(INITIAL_DEFAULT_SCHEMA, 3))
+ .doesNotThrowAnyException();
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2, 3})
+ public void testSupportedInitialDefault(int formatVersion) {
+ // only the initial default is a forward-incompatible change
Review Comment:
nit: We could make this self contained to the test by saying
Writer defaults are universally compatible
Or something?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API: Add compatibility checks for Schemas with default values [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer merged PR #11434:
URL: https://github.com/apache/iceberg/pull/11434
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org