Skip to content

Commit 0a21640

Browse files
nastranastra
committed
Core: Assign fresh IDs to view schema
1 parent 6f0d9dd commit 0a21640

File tree

4 files changed

+70
-19
lines changed

4 files changed

+70
-19
lines changed

‎core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java

+10
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.util.concurrent.Future;
3333
import java.util.concurrent.ScheduledExecutorService;
3434
import java.util.concurrent.TimeUnit;
35+
import java.util.concurrent.atomic.AtomicInteger;
3536
import java.util.function.BiFunction;
3637
import java.util.function.Function;
3738
import java.util.function.Supplier;
@@ -92,6 +93,7 @@
9293
import org.apache.iceberg.rest.responses.LoadViewResponse;
9394
import org.apache.iceberg.rest.responses.OAuthTokenResponse;
9495
import org.apache.iceberg.rest.responses.UpdateNamespacePropertiesResponse;
96+
import org.apache.iceberg.types.TypeUtil;
9597
import org.apache.iceberg.util.EnvironmentUtil;
9698
import org.apache.iceberg.util.Pair;
9799
import org.apache.iceberg.util.PropertyUtil;
@@ -1185,6 +1187,8 @@ public View create() {
11851187
Preconditions.checkState(
11861188
null != defaultNamespace, "Cannot create view without specifying a default namespace");
11871189

1190+
schema = TypeUtil.assignFreshIds(schema, new AtomicInteger(0)::incrementAndGet);
1191+
11881192
ViewVersion viewVersion =
11891193
ImmutableViewVersion.builder()
11901194
.versionId(1)
@@ -1262,6 +1266,12 @@ private View replace(LoadViewResponse response) {
12621266
.max(Integer::compareTo)
12631267
.orElseGet(metadata::currentVersionId);
12641268

1269+
schema =
1270+
TypeUtil.assignFreshIds(
1271+
schema,
1272+
metadata.schema(),
1273+
new AtomicInteger(metadata.schema().highestFieldId())::incrementAndGet);
1274+
12651275
ViewVersion viewVersion =
12661276
ImmutableViewVersion.builder()
12671277
.versionId(maxVersionId + 1)

‎core/src/main/java/org/apache/iceberg/view/BaseMetastoreViewCatalog.java

+10
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import java.util.List;
2222
import java.util.Map;
23+
import java.util.concurrent.atomic.AtomicInteger;
2324
import org.apache.iceberg.BaseMetastoreCatalog;
2425
import org.apache.iceberg.EnvironmentContext;
2526
import org.apache.iceberg.Schema;
@@ -33,6 +34,7 @@
3334
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
3435
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
3536
import org.apache.iceberg.relocated.com.google.common.collect.Maps;
37+
import org.apache.iceberg.types.TypeUtil;
3638

3739
public abstract class BaseMetastoreViewCatalog extends BaseMetastoreCatalog implements ViewCatalog {
3840
protected abstract ViewOperations newViewOps(TableIdentifier identifier);
@@ -155,6 +157,8 @@ private View create(ViewOperations ops) {
155157
Preconditions.checkState(
156158
null != defaultNamespace, "Cannot create view without specifying a default namespace");
157159

160+
schema = TypeUtil.assignFreshIds(schema, new AtomicInteger(0)::incrementAndGet);
161+
158162
ViewVersion viewVersion =
159163
ImmutableViewVersion.builder()
160164
.versionId(1)
@@ -204,6 +208,12 @@ private View replace(ViewOperations ops) {
204208
.max(Integer::compareTo)
205209
.orElseGet(metadata::currentVersionId);
206210

211+
schema =
212+
TypeUtil.assignFreshIds(
213+
schema,
214+
metadata.schema(),
215+
new AtomicInteger(metadata.schema().highestFieldId())::incrementAndGet);
216+
207217
ViewVersion viewVersion =
208218
ImmutableViewVersion.builder()
209219
.versionId(maxVersionId + 1)

‎core/src/main/java/org/apache/iceberg/view/ViewVersionReplace.java

+8
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@
2828
import static org.apache.iceberg.TableProperties.COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT;
2929

3030
import java.util.List;
31+
import java.util.concurrent.atomic.AtomicInteger;
3132
import org.apache.iceberg.EnvironmentContext;
3233
import org.apache.iceberg.Schema;
3334
import org.apache.iceberg.catalog.Namespace;
3435
import org.apache.iceberg.exceptions.CommitFailedException;
3536
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
3637
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
38+
import org.apache.iceberg.types.TypeUtil;
3739
import org.apache.iceberg.util.PropertyUtil;
3840
import org.apache.iceberg.util.Tasks;
3941

@@ -71,6 +73,12 @@ ViewMetadata internalApply() {
7173
.max(Integer::compareTo)
7274
.orElseGet(viewVersion::versionId);
7375

76+
schema =
77+
TypeUtil.assignFreshIds(
78+
schema,
79+
base.schema(),
80+
new AtomicInteger(base.schema().highestFieldId())::incrementAndGet);
81+
7482
ViewVersion newVersion =
7583
ImmutableViewVersion.builder()
7684
.versionId(maxVersionId + 1)

‎core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java

+42-19
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,20 @@ public abstract class ViewCatalogTests<C extends ViewCatalog & SupportsNamespace
5252
required(3, "id", Types.IntegerType.get(), "unique ID"),
5353
required(4, "data", Types.StringType.get()));
5454

55+
// actual schema for the view, with column IDs reassigned
56+
protected static final Schema VIEW_SCHEMA =
57+
new Schema(
58+
0,
59+
required(1, "id", Types.IntegerType.get(), "unique ID"),
60+
required(2, "data", Types.StringType.get()));
61+
5562
private static final Schema OTHER_SCHEMA =
5663
new Schema(7, required(1, "some_id", Types.IntegerType.get()));
5764

65+
// actual replaced schema for the view, with column IDs reassigned
66+
private static final Schema OTHER_VIEW_SCHEMA =
67+
new Schema(1, required(3, "some_id", Types.IntegerType.get()));
68+
5869
protected abstract C catalog();
5970

6071
protected abstract Catalog tableCatalog();
@@ -102,18 +113,18 @@ public void basicCreateView() {
102113
.first()
103114
.extracting(ViewHistoryEntry::versionId)
104115
.isEqualTo(1);
105-
assertThat(view.schema().schemaId()).isEqualTo(0);
106-
assertThat(view.schema().asStruct()).isEqualTo(SCHEMA.asStruct());
116+
assertThat(view.schema().schemaId()).isEqualTo(VIEW_SCHEMA.schemaId());
117+
assertThat(view.schema().asStruct()).isEqualTo(VIEW_SCHEMA.asStruct());
107118
assertThat(view.currentVersion().operation()).isEqualTo("create");
108-
assertThat(view.schemas()).hasSize(1).containsKey(0);
119+
assertThat(view.schemas()).hasSize(1).containsKey(VIEW_SCHEMA.schemaId());
109120
assertThat(view.versions()).hasSize(1).containsExactly(view.currentVersion());
110121

111122
assertThat(view.currentVersion())
112123
.isEqualTo(
113124
ImmutableViewVersion.builder()
114125
.timestampMillis(view.currentVersion().timestampMillis())
115126
.versionId(1)
116-
.schemaId(0)
127+
.schemaId(VIEW_SCHEMA.schemaId())
117128
.summary(view.currentVersion().summary())
118129
.defaultNamespace(identifier.namespace())
119130
.addRepresentations(
@@ -173,17 +184,17 @@ public void completeCreateView() {
173184
.extracting(ViewHistoryEntry::versionId)
174185
.isEqualTo(1);
175186
assertThat(view.currentVersion().operation()).isEqualTo("create");
176-
assertThat(view.schema().schemaId()).isEqualTo(0);
177-
assertThat(view.schema().asStruct()).isEqualTo(SCHEMA.asStruct());
178-
assertThat(view.schemas()).hasSize(1).containsKey(0);
187+
assertThat(view.schema().schemaId()).isEqualTo(VIEW_SCHEMA.schemaId());
188+
assertThat(view.schema().asStruct()).isEqualTo(VIEW_SCHEMA.asStruct());
189+
assertThat(view.schemas()).hasSize(1).containsKey(VIEW_SCHEMA.schemaId());
179190
assertThat(view.versions()).hasSize(1).containsExactly(view.currentVersion());
180191

181192
assertThat(view.currentVersion())
182193
.isEqualTo(
183194
ImmutableViewVersion.builder()
184195
.timestampMillis(view.currentVersion().timestampMillis())
185196
.versionId(1)
186-
.schemaId(0)
197+
.schemaId(VIEW_SCHEMA.schemaId())
187198
.summary(view.currentVersion().summary())
188199
.defaultNamespace(identifier.namespace())
189200
.defaultCatalog(catalog().name())
@@ -885,17 +896,20 @@ public void createOrReplaceView(boolean useCreateOrReplace) {
885896
.extracting(ViewHistoryEntry::versionId)
886897
.isEqualTo(2);
887898

888-
assertThat(replacedView.schema().schemaId()).isEqualTo(1);
889-
assertThat(replacedView.schema().asStruct()).isEqualTo(OTHER_SCHEMA.asStruct());
890-
assertThat(replacedView.schemas()).hasSize(2).containsKey(0).containsKey(1);
899+
assertThat(replacedView.schema().schemaId()).isEqualTo(OTHER_VIEW_SCHEMA.schemaId());
900+
assertThat(replacedView.schema().asStruct()).isEqualTo(OTHER_VIEW_SCHEMA.asStruct());
901+
assertThat(replacedView.schemas())
902+
.hasSize(2)
903+
.containsKey(VIEW_SCHEMA.schemaId())
904+
.containsKey(OTHER_VIEW_SCHEMA.schemaId());
891905

892906
ViewVersion replacedViewVersion = replacedView.currentVersion();
893907
assertThat(replacedView.versions())
894908
.hasSize(2)
895909
.containsExactly(viewVersion, replacedViewVersion);
896910
assertThat(replacedViewVersion).isNotNull();
897911
assertThat(replacedViewVersion.versionId()).isEqualTo(2);
898-
assertThat(replacedViewVersion.schemaId()).isEqualTo(1);
912+
assertThat(replacedViewVersion.schemaId()).isEqualTo(OTHER_VIEW_SCHEMA.schemaId());
899913
assertThat(replacedViewVersion.operation()).isEqualTo("replace");
900914
assertThat(replacedViewVersion.representations())
901915
.containsExactly(
@@ -1120,7 +1134,12 @@ public void replaceViewVersion() {
11201134
.element(1)
11211135
.extracting(ViewHistoryEntry::versionId)
11221136
.isEqualTo(updatedView.currentVersion().versionId());
1123-
assertThat(updatedView.schemas()).hasSize(2).containsKey(0).containsKey(1);
1137+
assertThat(updatedView.schemas())
1138+
.hasSize(2)
1139+
.containsKey(VIEW_SCHEMA.schemaId())
1140+
.containsKey(OTHER_VIEW_SCHEMA.schemaId());
1141+
assertThat(updatedView.schema().schemaId()).isEqualTo(OTHER_VIEW_SCHEMA.schemaId());
1142+
assertThat(updatedView.schema().asStruct()).isEqualTo(OTHER_VIEW_SCHEMA.asStruct());
11241143
assertThat(updatedView.versions())
11251144
.hasSize(2)
11261145
.containsExactly(viewVersion, updatedView.currentVersion());
@@ -1130,7 +1149,7 @@ public void replaceViewVersion() {
11301149
assertThat(updatedViewVersion.versionId()).isEqualTo(viewVersion.versionId() + 1);
11311150
assertThat(updatedViewVersion.operation()).isEqualTo("replace");
11321151
assertThat(updatedViewVersion.representations()).hasSize(1).containsExactly(trino);
1133-
assertThat(updatedViewVersion.schemaId()).isEqualTo(1);
1152+
assertThat(updatedViewVersion.schemaId()).isEqualTo(OTHER_VIEW_SCHEMA.schemaId());
11341153
assertThat(updatedViewVersion.defaultCatalog()).isEqualTo("default");
11351154
assertThat(updatedViewVersion.defaultNamespace()).isEqualTo(identifier.namespace());
11361155

@@ -1585,6 +1604,8 @@ public void concurrentReplaceViewVersion() {
15851604
viewOps.commit(current, sparkUpdate);
15861605

15871606
View updatedView = catalog().loadView(identifier);
1607+
assertThat(updatedView.schema().schemaId()).isEqualTo(VIEW_SCHEMA.schemaId());
1608+
assertThat(updatedView.schema().asStruct()).isEqualTo(VIEW_SCHEMA.asStruct());
15881609
ViewVersion viewVersion = updatedView.currentVersion();
15891610
assertThat(viewVersion.versionId()).isEqualTo(3);
15901611
assertThat(updatedView.versions()).hasSize(3);
@@ -1593,7 +1614,7 @@ public void concurrentReplaceViewVersion() {
15931614
ImmutableViewVersion.builder()
15941615
.timestampMillis(updatedView.version(1).timestampMillis())
15951616
.versionId(1)
1596-
.schemaId(0)
1617+
.schemaId(VIEW_SCHEMA.schemaId())
15971618
.summary(updatedView.version(1).summary())
15981619
.defaultNamespace(identifier.namespace())
15991620
.addRepresentations(
@@ -1608,7 +1629,7 @@ public void concurrentReplaceViewVersion() {
16081629
ImmutableViewVersion.builder()
16091630
.timestampMillis(updatedView.version(2).timestampMillis())
16101631
.versionId(2)
1611-
.schemaId(1)
1632+
.schemaId(OTHER_VIEW_SCHEMA.schemaId())
16121633
.summary(updatedView.version(2).summary())
16131634
.defaultNamespace(identifier.namespace())
16141635
.addRepresentations(
@@ -1623,7 +1644,7 @@ public void concurrentReplaceViewVersion() {
16231644
ImmutableViewVersion.builder()
16241645
.timestampMillis(updatedView.version(3).timestampMillis())
16251646
.versionId(3)
1626-
.schemaId(0)
1647+
.schemaId(VIEW_SCHEMA.schemaId())
16271648
.summary(updatedView.version(3).summary())
16281649
.defaultNamespace(identifier.namespace())
16291650
.addRepresentations(
@@ -1638,6 +1659,8 @@ public void concurrentReplaceViewVersion() {
16381659
.hasMessageContaining("Cannot commit");
16391660

16401661
View updatedView = catalog().loadView(identifier);
1662+
assertThat(updatedView.schema().schemaId()).isEqualTo(OTHER_VIEW_SCHEMA.schemaId());
1663+
assertThat(updatedView.schema().asStruct()).isEqualTo(OTHER_VIEW_SCHEMA.asStruct());
16411664
ViewVersion viewVersion = updatedView.currentVersion();
16421665
assertThat(viewVersion.versionId()).isEqualTo(2);
16431666
assertThat(updatedView.versions()).hasSize(2);
@@ -1646,7 +1669,7 @@ public void concurrentReplaceViewVersion() {
16461669
ImmutableViewVersion.builder()
16471670
.timestampMillis(updatedView.version(1).timestampMillis())
16481671
.versionId(1)
1649-
.schemaId(0)
1672+
.schemaId(VIEW_SCHEMA.schemaId())
16501673
.summary(updatedView.version(1).summary())
16511674
.defaultNamespace(identifier.namespace())
16521675
.addRepresentations(
@@ -1661,7 +1684,7 @@ public void concurrentReplaceViewVersion() {
16611684
ImmutableViewVersion.builder()
16621685
.timestampMillis(updatedView.version(2).timestampMillis())
16631686
.versionId(2)
1664-
.schemaId(1)
1687+
.schemaId(OTHER_VIEW_SCHEMA.schemaId())
16651688
.summary(updatedView.version(2).summary())
16661689
.defaultNamespace(identifier.namespace())
16671690
.addRepresentations(

0 commit comments

Comments
 (0)