nastra commented on code in PR #8909:
URL: https://github.com/apache/iceberg/pull/8909#discussion_r1387615484
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -378,27 +401,72 @@ public void renameTable(TableIdentifier from,
TableIdentifier to) {
// behavior. So better be safe than sorry.
}
+ private static void validateToContentForRename(
+ TableIdentifier from, TableIdentifier to, IcebergContent
existingToContent) {
+ if (existingToContent != null) {
+ if (existingToContent.getType() == Content.Type.ICEBERG_VIEW) {
+ throw new AlreadyExistsException("Cannot rename %s to %s. View already
exists", from, to);
+ } else if (existingToContent.getType() == Content.Type.ICEBERG_TABLE) {
+ throw new AlreadyExistsException("Cannot rename %s to %s. Table
already exists", from, to);
+ } else {
+ throw new AlreadyExistsException(
+ "Cannot rename %s to %s. Another content with same name already
exists", from, to);
+ }
+ }
+ }
+
+ private static void validateFromContentForRename(
+ TableIdentifier from, Content.Type type, IcebergContent
existingFromContent) {
+ if (existingFromContent == null) {
+ if (type == Content.Type.ICEBERG_VIEW) {
+ throw new NoSuchViewException("View does not exist: %s", from);
+ } else if (type == Content.Type.ICEBERG_TABLE) {
+ throw new NoSuchTableException("Table does not exist: %s", from);
+ } else {
+ throw new UnsupportedOperationException("Cannot rename for content
type: " + type);
Review Comment:
```suggestion
throw new UnsupportedOperationException("Cannot rename content type:
" + type);
```
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -378,27 +401,72 @@ public void renameTable(TableIdentifier from,
TableIdentifier to) {
// behavior. So better be safe than sorry.
}
+ private static void validateToContentForRename(
+ TableIdentifier from, TableIdentifier to, IcebergContent
existingToContent) {
+ if (existingToContent != null) {
+ if (existingToContent.getType() == Content.Type.ICEBERG_VIEW) {
+ throw new AlreadyExistsException("Cannot rename %s to %s. View already
exists", from, to);
+ } else if (existingToContent.getType() == Content.Type.ICEBERG_TABLE) {
+ throw new AlreadyExistsException("Cannot rename %s to %s. Table
already exists", from, to);
+ } else {
+ throw new AlreadyExistsException(
+ "Cannot rename %s to %s. Another content with same name already
exists", from, to);
+ }
+ }
+ }
+
+ private static void validateFromContentForRename(
+ TableIdentifier from, Content.Type type, IcebergContent
existingFromContent) {
+ if (existingFromContent == null) {
+ if (type == Content.Type.ICEBERG_VIEW) {
+ throw new NoSuchViewException("View does not exist: %s", from);
+ } else if (type == Content.Type.ICEBERG_TABLE) {
+ throw new NoSuchTableException("Table does not exist: %s", from);
+ } else {
+ throw new UnsupportedOperationException("Cannot rename for content
type: " + type);
+ }
+ } else if (existingFromContent.getType() != type) {
+ throw new UnsupportedOperationException(
+ "content type of from identifier should be of " + type);
Review Comment:
why not mention the actual identifier in the message here?
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -530,6 +599,17 @@ private String buildCommitMsg(TableMetadata base,
TableMetadata metadata, String
return String.format("Iceberg commit against %s", tableName);
}
+ private String buildCommitMsgForView(ViewMetadata base, ViewMetadata
metadata, String viewName) {
Review Comment:
```suggestion
private String buildCommitMsg(ViewMetadata base, ViewMetadata metadata,
String viewName) {
```
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieViewOperations.java:
##########
@@ -0,0 +1,164 @@
+/*
+ * 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.nessie;
+
+import java.util.Map;
+import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.exceptions.NoSuchViewException;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.view.BaseViewOperations;
+import org.apache.iceberg.view.ViewMetadata;
+import org.apache.iceberg.view.ViewMetadataParser;
+import org.projectnessie.client.http.HttpClientException;
+import org.projectnessie.error.NessieConflictException;
+import org.projectnessie.error.NessieNotFoundException;
+import org.projectnessie.model.Content;
+import org.projectnessie.model.ContentKey;
+import org.projectnessie.model.IcebergTable;
+import org.projectnessie.model.IcebergView;
+import org.projectnessie.model.Reference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NessieViewOperations extends BaseViewOperations {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(NessieViewOperations.class);
+
+ private final NessieIcebergClient client;
+ private final ContentKey key;
+ private final FileIO fileIO;
+ private final Map<String, String> catalogOptions;
+ private IcebergView icebergView;
+
+ NessieViewOperations(
+ ContentKey key,
+ NessieIcebergClient client,
+ FileIO fileIO,
+ Map<String, String> catalogOptions) {
+ this.key = key;
+ this.client = client;
+ this.fileIO = fileIO;
+ this.catalogOptions = catalogOptions;
+ }
+
+ @Override
+ public void doRefresh() {
+ try {
+ client.refresh();
+ } catch (NessieNotFoundException e) {
+ throw new RuntimeException(
+ String.format(
+ "Failed to refresh as ref '%s' is no longer valid.",
client.getRef().getName()),
+ e);
+ }
+ String metadataLocation = null;
+ Reference reference = client.getRef().getReference();
+ try {
+ Content content =
client.getApi().getContent().key(key).reference(reference).get().get(key);
+ LOG.debug("Content '{}' at '{}': {}", key, reference, content);
+ if (content == null) {
+ if (currentMetadataLocation() != null) {
+ throw new NoSuchViewException("View does not exist: %s in %s", key,
reference);
+ }
+ } else {
+ this.icebergView =
+ content
+ .unwrap(IcebergView.class)
+ .orElseThrow(
+ () -> {
+ if (content instanceof IcebergTable) {
+ return new AlreadyExistsException(
+ "Table with same name already exists: %s in %s",
key, reference);
+ } else {
+ return new IllegalStateException(
+ String.format(
+ "Cannot refresh Iceberg view: Nessie points to
a non-Iceberg object for path: %s.",
+ key));
+ }
+ });
+ metadataLocation = icebergView.getMetadataLocation();
+ }
+ } catch (NessieNotFoundException ex) {
+ if (currentMetadataLocation() != null) {
+ throw new NoSuchViewException("View does not exist: %s in %s", key,
reference);
+ }
+ }
+ refreshFromMetadataLocation(metadataLocation, null, 2, l ->
loadViewMetadata(l, reference));
+ }
+
+ private ViewMetadata loadViewMetadata(String metadataLocation, Reference
reference) {
Review Comment:
does this need to be moved to `NessieUtil` so that it can be used from
Trino? (similar to https://github.com/apache/iceberg/pull/7893)
##########
nessie/src/test/java/org/apache/iceberg/nessie/TestNessieViewCatalog.java:
##########
@@ -0,0 +1,222 @@
+/*
+ * 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.nessie;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Path;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.view.BaseView;
+import org.apache.iceberg.view.View;
+import org.apache.iceberg.view.ViewCatalogTests;
+import org.apache.iceberg.view.ViewMetadata;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.io.TempDir;
+import org.projectnessie.client.api.NessieApiV1;
+import org.projectnessie.client.ext.NessieApiVersion;
+import org.projectnessie.client.ext.NessieApiVersions;
+import org.projectnessie.client.ext.NessieClientFactory;
+import org.projectnessie.client.ext.NessieClientUri;
+import org.projectnessie.error.NessieConflictException;
+import org.projectnessie.error.NessieNotFoundException;
+import org.projectnessie.jaxrs.ext.NessieJaxRsExtension;
+import org.projectnessie.model.Branch;
+import org.projectnessie.model.Reference;
+import org.projectnessie.model.Tag;
+import org.projectnessie.versioned.storage.common.persist.Persist;
+import org.projectnessie.versioned.storage.inmemory.InmemoryBackendTestFactory;
+import org.projectnessie.versioned.storage.testextension.NessieBackend;
+import org.projectnessie.versioned.storage.testextension.NessiePersist;
+import org.projectnessie.versioned.storage.testextension.PersistExtension;
+
+@ExtendWith(PersistExtension.class)
+@NessieBackend(InmemoryBackendTestFactory.class)
+@NessieApiVersions // test all versions
+public class TestNessieViewCatalog extends ViewCatalogTests<NessieCatalog> {
+
+ @NessiePersist static Persist persist;
+
+ @RegisterExtension
+ static NessieJaxRsExtension server = NessieJaxRsExtension.jaxRsExtension(()
-> persist);
+
+ @TempDir public Path temp;
+
+ private NessieCatalog catalog;
+ private NessieApiV1 api;
+ private NessieApiVersion apiVersion;
+ private Configuration hadoopConfig;
+ private String initialHashOfDefaultBranch;
+ private String uri;
+
+ @BeforeEach
+ public void setUp(NessieClientFactory clientFactory, @NessieClientUri URI
nessieUri)
+ throws NessieNotFoundException {
+ api = clientFactory.make();
+ apiVersion = clientFactory.apiVersion();
+ initialHashOfDefaultBranch = api.getDefaultBranch().getHash();
+ uri = nessieUri.toASCIIString();
+ hadoopConfig = new Configuration();
+ catalog = initNessieCatalog("main");
+ }
+
+ @AfterEach
+ public void afterEach() throws IOException {
+ resetData();
+ try {
+ if (catalog != null) {
+ catalog.close();
+ }
+ api.close();
+ } finally {
+ catalog = null;
+ api = null;
+ hadoopConfig = null;
+ }
+ }
+
+ private void resetData() throws NessieConflictException,
NessieNotFoundException {
+ Branch defaultBranch = api.getDefaultBranch();
+ for (Reference r : api.getAllReferences().get().getReferences()) {
+ if (r instanceof Branch && !r.getName().equals(defaultBranch.getName()))
{
+ api.deleteBranch().branch((Branch) r).delete();
+ }
+ if (r instanceof Tag) {
+ api.deleteTag().tag((Tag) r).delete();
+ }
+ }
+ api.assignBranch()
+ .assignTo(Branch.of(defaultBranch.getName(),
initialHashOfDefaultBranch))
+ .branch(defaultBranch)
+ .assign();
+ }
+
+ private NessieCatalog initNessieCatalog(String ref) {
+ NessieCatalog newCatalog = new NessieCatalog();
+ newCatalog.setConf(hadoopConfig);
+ ImmutableMap<String, String> options =
+ ImmutableMap.of(
+ "ref",
+ ref,
+ CatalogProperties.URI,
+ uri,
+ CatalogProperties.WAREHOUSE_LOCATION,
+ temp.toUri().toString(),
+ "client-api-version",
+ apiVersion == NessieApiVersion.V2 ? "2" : "1");
+ newCatalog.initialize("nessie", options);
+ return newCatalog;
+ }
+
+ @Override
+ protected NessieCatalog catalog() {
+ return catalog;
+ }
+
+ @Override
+ protected Catalog tableCatalog() {
+ return catalog;
+ }
+
+ @Override
+ protected boolean requiresNamespaceCreate() {
+ return true;
+ }
+
+ @Override
+ public void renameViewNamespaceMissing() {
+ // Need a fix from Nessie
(https://github.com/projectnessie/nessie/issues/7645)
+ }
+
+ // Overriding the below rename view testcases to exclude checking same view
metadata after rename.
Review Comment:
I think that would be ok to change the semantics to `contains all the
original fields but may also contain some extra ones (in case a catalog impl
decides to add new properties)`
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -530,6 +599,17 @@ private String buildCommitMsg(TableMetadata base,
TableMetadata metadata, String
return String.format("Iceberg commit against %s", tableName);
}
+ private String buildCommitMsgForView(ViewMetadata base, ViewMetadata
metadata, String viewName) {
Review Comment:
mainly because the `view` part is already implicit through the method
parameters
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -312,26 +327,33 @@ namespace, getRef().getName()),
}
public void renameTable(TableIdentifier from, TableIdentifier to) {
+ renameContent(from, to, Content.Type.ICEBERG_TABLE);
+ }
+
+ public void renameView(TableIdentifier from, TableIdentifier to) {
+ renameContent(from, to, Content.Type.ICEBERG_VIEW);
+ }
+
+ private void renameContent(TableIdentifier from, TableIdentifier to,
Content.Type type) {
getRef().checkMutable();
- IcebergTable existingFromTable = table(from);
- if (existingFromTable == null) {
- throw new NoSuchTableException("Table does not exist: %s", from.name());
- }
- IcebergTable existingToTable = table(to);
- if (existingToTable != null) {
- throw new AlreadyExistsException("Table already exists: %s", to.name());
- }
+ IcebergContent existingFromContent = asContent(from, IcebergContent.class);
+ validateFromContentForRename(from, type, existingFromContent);
+
+ IcebergContent existingToContent = asContent(to, IcebergContent.class);
+ validateToContentForRename(from, to, existingToContent);
+ String contentTypeString = type == Content.Type.ICEBERG_VIEW ? "view" :
"table";
Review Comment:
nit: maybe rename to `contentType` instead of using the `String` suffix,
since you're already using the same var name in `dropContent()`
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieUtil.java:
##########
@@ -165,4 +179,74 @@ public static TableMetadata
updateTableMetadataWithNessieSpecificProperties(
return builder.discardChanges().build();
}
+
+ static void handleExceptionsForCommits(Exception exception, String refName,
Content.Type type) {
+ if (exception instanceof NessieConflictException) {
+ if (exception instanceof NessieReferenceConflictException) {
+ // Throws a specialized exception, if possible
+ NessieUtil.maybeThrowSpecializedException(
+ (NessieReferenceConflictException) exception, type);
+ }
+
+ throw new CommitFailedException(
+ exception,
+ "Cannot commit: Reference hash is out of date. "
Review Comment:
```suggestion
"Cannot commit: Reference hash is out of date. Update the
reference '%s' and try again",
```
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieViewOperations.java:
##########
@@ -0,0 +1,164 @@
+/*
+ * 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.nessie;
+
+import java.util.Map;
+import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.exceptions.NoSuchViewException;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.view.BaseViewOperations;
+import org.apache.iceberg.view.ViewMetadata;
+import org.apache.iceberg.view.ViewMetadataParser;
+import org.projectnessie.client.http.HttpClientException;
+import org.projectnessie.error.NessieConflictException;
+import org.projectnessie.error.NessieNotFoundException;
+import org.projectnessie.model.Content;
+import org.projectnessie.model.ContentKey;
+import org.projectnessie.model.IcebergTable;
+import org.projectnessie.model.IcebergView;
+import org.projectnessie.model.Reference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NessieViewOperations extends BaseViewOperations {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(NessieViewOperations.class);
+
+ private final NessieIcebergClient client;
+ private final ContentKey key;
+ private final FileIO fileIO;
+ private final Map<String, String> catalogOptions;
+ private IcebergView icebergView;
+
+ NessieViewOperations(
+ ContentKey key,
+ NessieIcebergClient client,
+ FileIO fileIO,
+ Map<String, String> catalogOptions) {
+ this.key = key;
+ this.client = client;
+ this.fileIO = fileIO;
+ this.catalogOptions = catalogOptions;
+ }
+
+ @Override
+ public void doRefresh() {
+ try {
+ client.refresh();
+ } catch (NessieNotFoundException e) {
+ throw new RuntimeException(
+ String.format(
+ "Failed to refresh as ref '%s' is no longer valid.",
client.getRef().getName()),
+ e);
+ }
+ String metadataLocation = null;
+ Reference reference = client.getRef().getReference();
+ try {
+ Content content =
client.getApi().getContent().key(key).reference(reference).get().get(key);
+ LOG.debug("Content '{}' at '{}': {}", key, reference, content);
+ if (content == null) {
+ if (currentMetadataLocation() != null) {
+ throw new NoSuchViewException("View does not exist: %s in %s", key,
reference);
+ }
+ } else {
+ this.icebergView =
+ content
+ .unwrap(IcebergView.class)
+ .orElseThrow(
+ () -> {
+ if (content instanceof IcebergTable) {
+ return new AlreadyExistsException(
+ "Table with same name already exists: %s in %s",
key, reference);
+ } else {
+ return new IllegalStateException(
+ String.format(
+ "Cannot refresh Iceberg view: Nessie points to
a non-Iceberg object for path: %s.",
+ key));
+ }
+ });
+ metadataLocation = icebergView.getMetadataLocation();
+ }
+ } catch (NessieNotFoundException ex) {
+ if (currentMetadataLocation() != null) {
+ throw new NoSuchViewException("View does not exist: %s in %s", key,
reference);
+ }
+ }
+ refreshFromMetadataLocation(metadataLocation, null, 2, l ->
loadViewMetadata(l, reference));
+ }
+
+ private ViewMetadata loadViewMetadata(String metadataLocation, Reference
reference) {
+ ViewMetadata metadata =
ViewMetadataParser.read(io().newInputFile(metadataLocation));
+ Map<String, String> newProperties = Maps.newHashMap(metadata.properties());
+ newProperties.put(NessieTableOperations.NESSIE_COMMIT_ID_PROPERTY,
reference.getHash());
+
+ return ViewMetadata.buildFrom(
+
ViewMetadata.buildFrom(metadata).setProperties(newProperties).build())
+ .setMetadataLocation(metadataLocation)
+ .build();
+ }
+
+ @Override
+ public void doCommit(ViewMetadata base, ViewMetadata metadata) {
+
Review Comment:
unnecessary newline
##########
nessie/src/test/java/org/apache/iceberg/nessie/BaseTestIceberg.java:
##########
@@ -247,6 +283,14 @@ static String metadataLocation(NessieCatalog catalog,
TableIdentifier tableIdent
return icebergOps.currentMetadataLocation();
}
+ static String viewMetadataLocation(NessieCatalog catalog, TableIdentifier
tableIdentifier) {
+ View view = catalog.loadView(tableIdentifier);
+ BaseView baseView = (BaseView) view;
+ ViewOperations ops = baseView.operations();
+ NessieViewOperations icebergOps = (NessieViewOperations) ops;
+ return icebergOps.current().metadataFileLocation();
Review Comment:
could also be done in a single line: `return ((BaseView)
catalog.loadView(tableIdentifier)).operations().current().metadataFileLocation();`
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -530,6 +599,17 @@ private String buildCommitMsg(TableMetadata base,
TableMetadata metadata, String
return String.format("Iceberg commit against %s", tableName);
}
+ private String buildCommitMsgForView(ViewMetadata base, ViewMetadata
metadata, String viewName) {
+ String operation = metadata.currentVersion().operation();
+ if (base != null &&
!metadata.currentSchemaId().equals(base.currentSchemaId())) {
+ return String.format(
+ "Iceberg schema change against %s for the operation %s", viewName,
operation);
+ } else if (base == null) {
+ return String.format("Iceberg view created with name %s", viewName);
+ }
+ return String.format("Iceberg commit against %s for the operation %s",
viewName, operation);
Review Comment:
rather than having two cases for a create/replace, you could just use the
`operation` from above: `return String.format("Iceberg view %sd with name %s",
operation, viewName);`
##########
nessie/src/test/java/org/apache/iceberg/nessie/TestBranchVisibilityForView.java:
##########
@@ -0,0 +1,291 @@
+/*
+ * 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.nessie;
+
+import static org.apache.iceberg.types.Types.NestedField.required;
+
+import java.util.Arrays;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.view.BaseView;
+import org.apache.iceberg.view.View;
+import org.assertj.core.api.AbstractStringAssert;
+import org.assertj.core.api.Assertions;
+import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.projectnessie.error.NessieConflictException;
+import org.projectnessie.error.NessieNotFoundException;
+import org.projectnessie.model.Branch;
+import org.projectnessie.model.Reference;
+
+public class TestBranchVisibilityForView extends BaseTestIceberg {
+
+ private final TableIdentifier viewIdentifier1 =
TableIdentifier.of("test-ns", "view1");
+ private final TableIdentifier viewIdentifier2 =
TableIdentifier.of("test-ns", "view2");
+ private NessieCatalog testCatalog;
+
+ public TestBranchVisibilityForView() {
+ super("main");
+ }
+
+ @BeforeEach
+ public void before() throws NessieNotFoundException, NessieConflictException
{
+ createView(catalog, viewIdentifier1);
+ createView(catalog, viewIdentifier2);
+ createBranch("test");
+ testCatalog = initCatalog("test");
+ }
+
+ @AfterEach
+ public void after() throws NessieNotFoundException, NessieConflictException {
+ catalog.dropView(viewIdentifier1);
+ catalog.dropView(viewIdentifier2);
+ for (Reference reference : api.getAllReferences().get().getReferences()) {
+ if (!reference.getName().equals("main")) {
+ api.deleteBranch().branch((Branch) reference).delete();
+ }
+ }
+ testCatalog = null;
+ }
+
+ @Test
+ public void testBranchNoChange() {
+ testCatalogEquality(catalog, testCatalog, true, true, () -> {});
+ }
+
+ /** Ensure catalogs can't see each others updates. */
+ @Test
+ public void testUpdateCatalogs() {
+ testCatalogEquality(
+ catalog, testCatalog, false, true, () -> replaceView(catalog,
viewIdentifier1));
+
+ testCatalogEquality(
+ catalog, testCatalog, false, false, () -> replaceView(catalog,
viewIdentifier2));
+ }
+
+ @Test
+ public void testCatalogOnReference() {
+ replaceView(catalog, viewIdentifier1);
+ replaceView(testCatalog, viewIdentifier2);
+
+ // catalog created with ref points to same catalog as above
+ NessieCatalog refCatalog = initCatalog("test");
+ testCatalogEquality(refCatalog, testCatalog, true, true, () -> {});
+
+ // catalog created with hash points to same catalog as above
+ NessieCatalog refHashCatalog = initCatalog("main");
+ testCatalogEquality(refHashCatalog, catalog, true, true, () -> {});
+ }
+
+ @Test
+ public void testCatalogWithViewNames() {
+ replaceView(testCatalog, viewIdentifier2);
+
+ String mainName = "main";
+
+ // asking for view@branch gives expected regardless of catalog
+ Assertions.assertThat(
+ viewMetadataLocation(catalog, TableIdentifier.of("test-ns",
"view1@test")))
+ .isEqualTo(viewMetadataLocation(testCatalog, viewIdentifier1));
+
+ // Asking for view@branch gives expected regardless of catalog.
+ // Earlier versions used "view1@" +
tree.getReferenceByName("main").getHash() before, but since
+ // Nessie 0.8.2 the branch name became mandatory and specifying a hash
within a branch is not
+ // possible.
+ Assertions.assertThat(
+ viewMetadataLocation(catalog, TableIdentifier.of("test-ns",
"view1@" + mainName)))
+ .isEqualTo(viewMetadataLocation(testCatalog, viewIdentifier1));
+ }
+
+ @Test
+ public void testConcurrentChanges() {
+ NessieCatalog emptyTestCatalog = initCatalog("test");
+ replaceView(testCatalog, viewIdentifier1);
+ // Updating view with out of date hash. We expect this to succeed because
of retry despite the
Review Comment:
```suggestion
// Updating view with out-of-date hash. We expect this to succeed
because of retry despite the
```
##########
nessie/src/test/java/org/apache/iceberg/nessie/BaseTestIceberg.java:
##########
@@ -247,6 +283,14 @@ static String metadataLocation(NessieCatalog catalog,
TableIdentifier tableIdent
return icebergOps.currentMetadataLocation();
}
+ static String viewMetadataLocation(NessieCatalog catalog, TableIdentifier
tableIdentifier) {
Review Comment:
```suggestion
static String viewMetadataLocation(NessieCatalog catalog, TableIdentifier
identifier) {
```
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -540,4 +630,72 @@ public void close() {
api.close();
}
}
+
+ public void commitView(
+ ViewMetadata base,
+ ViewMetadata metadata,
+ String newMetadataLocation,
+ String contentId,
+ ContentKey key)
+ throws NessieConflictException, NessieNotFoundException {
+ UpdateableReference updateableReference = getRef();
+
+ updateableReference.checkMutable();
+
+ Branch current = (Branch) updateableReference.getReference();
+ Branch expectedHead = current;
+ if (base != null) {
+ String metadataCommitId =
+ base.properties()
+ .getOrDefault(
+ NessieTableOperations.NESSIE_COMMIT_ID_PROPERTY,
expectedHead.getHash());
+ if (metadataCommitId != null) {
+ expectedHead = Branch.of(expectedHead.getName(), metadataCommitId);
+ }
+ }
+
+ long versionId = metadata.currentVersion().versionId();
+
+ ImmutableIcebergView.Builder newViewBuilder =
ImmutableIcebergView.builder();
+ // Directly casting to `SQLViewRepresentation` as only SQL type exist in
+ // `ViewRepresentation.Type`.
+ // Assuming only one engine's dialect will be used, Nessie IcebergView
currently holds one
+ // representation.
+ // View loaded from catalog will have all the representation as it parses
the view metadata
+ // file.
+ SQLViewRepresentation sqlViewRepresentation =
+ (SQLViewRepresentation)
metadata.currentVersion().representations().get(0);
Review Comment:
I think this is the last major point that needs to be adressed before this
PR can go in
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -378,27 +401,72 @@ public void renameTable(TableIdentifier from,
TableIdentifier to) {
// behavior. So better be safe than sorry.
}
+ private static void validateToContentForRename(
+ TableIdentifier from, TableIdentifier to, IcebergContent
existingToContent) {
+ if (existingToContent != null) {
+ if (existingToContent.getType() == Content.Type.ICEBERG_VIEW) {
+ throw new AlreadyExistsException("Cannot rename %s to %s. View already
exists", from, to);
+ } else if (existingToContent.getType() == Content.Type.ICEBERG_TABLE) {
+ throw new AlreadyExistsException("Cannot rename %s to %s. Table
already exists", from, to);
+ } else {
+ throw new AlreadyExistsException(
+ "Cannot rename %s to %s. Another content with same name already
exists", from, to);
+ }
+ }
+ }
+
+ private static void validateFromContentForRename(
+ TableIdentifier from, Content.Type type, IcebergContent
existingFromContent) {
+ if (existingFromContent == null) {
+ if (type == Content.Type.ICEBERG_VIEW) {
+ throw new NoSuchViewException("View does not exist: %s", from);
+ } else if (type == Content.Type.ICEBERG_TABLE) {
+ throw new NoSuchTableException("Table does not exist: %s", from);
+ } else {
+ throw new UnsupportedOperationException("Cannot rename for content
type: " + type);
Review Comment:
or maybe `Cannot perform rename for content type...`
##########
nessie/src/test/java/org/apache/iceberg/nessie/TestNessieView.java:
##########
@@ -0,0 +1,337 @@
+/*
+ * 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.nessie;
+
+import static org.apache.iceberg.types.Types.NestedField.optional;
+import static org.apache.iceberg.types.Types.NestedField.required;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.view.SQLViewRepresentation;
+import org.apache.iceberg.view.View;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.projectnessie.client.ext.NessieClientFactory;
+import org.projectnessie.client.ext.NessieClientUri;
+import org.projectnessie.error.NessieNotFoundException;
+import org.projectnessie.model.Branch;
+import org.projectnessie.model.CommitMeta;
+import org.projectnessie.model.ContentKey;
+import org.projectnessie.model.IcebergView;
+import org.projectnessie.model.ImmutableTableReference;
+import org.projectnessie.model.LogResponse.LogEntry;
+
+public class TestNessieView extends BaseTestIceberg {
+
+ private static final String BRANCH = "iceberg-view-test";
+
+ private static final String DB_NAME = "db";
+ private static final String VIEW_NAME = "view";
+ private static final TableIdentifier VIEW_IDENTIFIER =
TableIdentifier.of(DB_NAME, VIEW_NAME);
+ private static final ContentKey KEY = ContentKey.of(DB_NAME, VIEW_NAME);
+ private static final Schema SCHEMA =
+ new Schema(Types.StructType.of(required(1, "id",
Types.LongType.get())).fields());
+ private static final Schema ALTERED =
+ new Schema(
+ Types.StructType.of(
+ required(1, "id", Types.LongType.get()),
+ optional(2, "data", Types.LongType.get()))
+ .fields());
+
+ private String viewLocation;
+
+ public TestNessieView() {
+ super(BRANCH);
+ }
+
+ @Override
+ @BeforeEach
+ public void beforeEach(NessieClientFactory clientFactory, @NessieClientUri
URI nessieUri)
+ throws IOException {
+ super.beforeEach(clientFactory, nessieUri);
+ this.viewLocation =
+ createView(catalog, VIEW_IDENTIFIER,
SCHEMA).location().replaceFirst("file:", "");
+ }
+
+ @Override
+ @AfterEach
+ public void afterEach() throws Exception {
+ // drop the view data
+ if (viewLocation != null) {
+ try (Stream<Path> walk = Files.walk(Paths.get(viewLocation))) {
+
walk.sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete);
+ }
+ catalog.dropView(VIEW_IDENTIFIER);
+ }
+
+ super.afterEach();
+ }
+
+ private IcebergView getView(ContentKey key) throws NessieNotFoundException {
+ return getView(BRANCH, key);
+ }
+
+ private IcebergView getView(String ref, ContentKey key) throws
NessieNotFoundException {
+ return
api.getContent().key(key).refName(ref).get().get(key).unwrap(IcebergView.class).get();
+ }
+
+ /** Verify that Nessie always returns the globally-current global-content w/
only DMLs. */
+ @Test
+ public void verifyStateMovesForDML() throws Exception {
+ // 1. initialize view
+ View icebergView = catalog.loadView(VIEW_IDENTIFIER);
+ icebergView
+ .replaceVersion()
+ .withQuery("spark", "some query")
+ .withSchema(SCHEMA)
+ .withDefaultNamespace(VIEW_IDENTIFIER.namespace())
+ .commit();
+
+ // 2. create 2nd branch
+ String testCaseBranch = "verify-global-moving";
+ api.createReference()
+ .sourceRefName(BRANCH)
+ .reference(Branch.of(testCaseBranch, catalog.currentHash()))
+ .create();
+ IcebergView contentInitialMain = getView(BRANCH, KEY);
+ IcebergView contentInitialBranch = getView(testCaseBranch, KEY);
+ View viewInitialMain = catalog.loadView(VIEW_IDENTIFIER);
+
+ // verify view-metadata-location + version-id
+ Assertions.assertThat(contentInitialMain)
+ .as("global-contents + snapshot-id equal on both branches in Nessie")
+ .isEqualTo(contentInitialBranch);
+ Assertions.assertThat(viewInitialMain.currentVersion()).isNotNull();
+
+ // 3. modify view in "main" branch
+
+ icebergView
+ .replaceVersion()
+ .withQuery("trino", "some other query")
+ .withSchema(SCHEMA)
+ .withDefaultNamespace(VIEW_IDENTIFIER.namespace())
+ .commit();
+
+ IcebergView contentsAfter1Main = getView(KEY);
+ IcebergView contentsAfter1Branch = getView(testCaseBranch, KEY);
+ View viewAfter1Main = catalog.loadView(VIEW_IDENTIFIER);
+
+ // --> assert getValue() against both branches returns the updated
metadata-location
+ // verify view-metadata-location
+ Assertions.assertThat(contentInitialMain.getMetadataLocation())
+ .describedAs("metadata-location must change on %s", BRANCH)
+ .isNotEqualTo(contentsAfter1Main.getMetadataLocation());
+ Assertions.assertThat(contentInitialBranch.getMetadataLocation())
+ .describedAs("metadata-location must not change on %s", testCaseBranch)
+ .isEqualTo(contentsAfter1Branch.getMetadataLocation());
+ Assertions.assertThat(contentsAfter1Main)
+ .extracting(IcebergView::getSchemaId)
+ .describedAs("schema ID must be same across branches")
+ .isEqualTo(contentsAfter1Branch.getSchemaId());
+ // verify updates
+ Assertions.assertThat(
+ ((SQLViewRepresentation)
viewAfter1Main.currentVersion().representations().get(0))
+ .dialect())
+ .isEqualTo("trino");
+
+ // 4. modify view in "main" branch again
+
+ icebergView
+ .replaceVersion()
+ .withQuery("flink", "some query")
+ .withSchema(SCHEMA)
+ .withDefaultNamespace(VIEW_IDENTIFIER.namespace())
+ .commit();
+
+ IcebergView contentsAfter2Main = getView(KEY);
+ IcebergView contentsAfter2Branch = getView(testCaseBranch, KEY);
+ View viewAfter2Main = catalog.loadView(VIEW_IDENTIFIER);
+
+ // --> assert getValue() against both branches returns the updated
metadata-location
+ // verify view-metadata-location
+ Assertions.assertThat(contentsAfter2Main.getMetadataLocation())
+ .describedAs("metadata-location must change on %s", BRANCH)
+ .isNotEqualTo(contentsAfter1Main.getMetadataLocation());
+ Assertions.assertThat(contentsAfter2Branch.getMetadataLocation())
+ .describedAs("on-reference-state must not change on %s",
testCaseBranch)
+ .isEqualTo(contentsAfter1Branch.getMetadataLocation());
+ Assertions.assertThat(
+ ((SQLViewRepresentation)
viewAfter2Main.currentVersion().representations().get(0))
+ .dialect())
+ .isEqualTo("flink");
+ }
+
+ @Test
+ public void testUpdate() throws IOException {
+ String viewName = VIEW_IDENTIFIER.name();
+ View icebergView = catalog.loadView(VIEW_IDENTIFIER);
+ // add a column
+ icebergView
+ .replaceVersion()
+ .withQuery("spark", "some query")
+ .withSchema(ALTERED)
+ .withDefaultNamespace(VIEW_IDENTIFIER.namespace())
+ .commit();
+
+ getView(KEY); // sanity, check view exists
+ // check parameters are in expected state
+ String expected = temp.toUri() + DB_NAME + "/" + viewName;
+ Assertions.assertThat(getViewBasePath(viewName)).isEqualTo(expected);
+
+
Assertions.assertThat(metadataVersionFiles(viewLocation)).isNotNull().hasSize(2);
+
+ verifyCommitMetadata();
+ }
+
+ @Test
+ public void testRenameWithTableReference() throws NessieNotFoundException {
+ String renamedViewName = "rename_view_name";
+ TableIdentifier renameViewIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(), renamedViewName);
+
+ ImmutableTableReference fromTableReference =
+ ImmutableTableReference.builder()
+ .reference(catalog.currentRefName())
+ .name(VIEW_IDENTIFIER.name())
+ .build();
+ ImmutableTableReference toTableReference =
+ ImmutableTableReference.builder()
+ .reference(catalog.currentRefName())
+ .name(renameViewIdentifier.name())
+ .build();
+ TableIdentifier fromIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(),
fromTableReference.toString());
+ TableIdentifier toIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(),
toTableReference.toString());
+
+ catalog.renameView(fromIdentifier, toIdentifier);
+ Assertions.assertThat(catalog.viewExists(fromIdentifier)).isFalse();
+ Assertions.assertThat(catalog.viewExists(toIdentifier)).isTrue();
+
+ Assertions.assertThat(catalog.dropView(toIdentifier)).isTrue();
+
+ verifyCommitMetadata();
+ }
+
+ @Test
+ public void testRenameWithTableReferenceInvalidCase() {
+ String renamedViewName = "rename_view_name";
+ TableIdentifier renameViewIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(), renamedViewName);
+
+ ImmutableTableReference fromTableReference =
+ ImmutableTableReference.builder()
+ .reference("Something")
+ .name(VIEW_IDENTIFIER.name())
+ .build();
+ ImmutableTableReference toTableReference =
+ ImmutableTableReference.builder()
+ .reference(catalog.currentRefName())
+ .name(renameViewIdentifier.name())
+ .build();
+ TableIdentifier fromIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(),
fromTableReference.toString());
+ TableIdentifier toIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(),
toTableReference.toString());
+
+ Assertions.assertThatThrownBy(() -> catalog.renameView(fromIdentifier,
toIdentifier))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(
+ "Cannot rename ICEBERG_VIEW 'view' on reference 'Something' to
'rename_view_name' on reference 'iceberg-view-test' : source and target
references must be the same.");
Review Comment:
there is an unnecessary space. The same is for the error message for
`ICEBERG_TABLE`
##########
nessie/src/test/java/org/apache/iceberg/nessie/TestNessieView.java:
##########
@@ -0,0 +1,337 @@
+/*
+ * 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.nessie;
+
+import static org.apache.iceberg.types.Types.NestedField.optional;
+import static org.apache.iceberg.types.Types.NestedField.required;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.view.SQLViewRepresentation;
+import org.apache.iceberg.view.View;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.projectnessie.client.ext.NessieClientFactory;
+import org.projectnessie.client.ext.NessieClientUri;
+import org.projectnessie.error.NessieNotFoundException;
+import org.projectnessie.model.Branch;
+import org.projectnessie.model.CommitMeta;
+import org.projectnessie.model.ContentKey;
+import org.projectnessie.model.IcebergView;
+import org.projectnessie.model.ImmutableTableReference;
+import org.projectnessie.model.LogResponse.LogEntry;
+
+public class TestNessieView extends BaseTestIceberg {
+
+ private static final String BRANCH = "iceberg-view-test";
+
+ private static final String DB_NAME = "db";
+ private static final String VIEW_NAME = "view";
+ private static final TableIdentifier VIEW_IDENTIFIER =
TableIdentifier.of(DB_NAME, VIEW_NAME);
+ private static final ContentKey KEY = ContentKey.of(DB_NAME, VIEW_NAME);
+ private static final Schema SCHEMA =
+ new Schema(Types.StructType.of(required(1, "id",
Types.LongType.get())).fields());
+ private static final Schema ALTERED =
+ new Schema(
+ Types.StructType.of(
+ required(1, "id", Types.LongType.get()),
+ optional(2, "data", Types.LongType.get()))
+ .fields());
+
+ private String viewLocation;
+
+ public TestNessieView() {
+ super(BRANCH);
+ }
+
+ @Override
+ @BeforeEach
+ public void beforeEach(NessieClientFactory clientFactory, @NessieClientUri
URI nessieUri)
+ throws IOException {
+ super.beforeEach(clientFactory, nessieUri);
+ this.viewLocation =
+ createView(catalog, VIEW_IDENTIFIER,
SCHEMA).location().replaceFirst("file:", "");
+ }
+
+ @Override
+ @AfterEach
+ public void afterEach() throws Exception {
+ // drop the view data
+ if (viewLocation != null) {
+ try (Stream<Path> walk = Files.walk(Paths.get(viewLocation))) {
+
walk.sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete);
+ }
+ catalog.dropView(VIEW_IDENTIFIER);
+ }
+
+ super.afterEach();
+ }
+
+ private IcebergView getView(ContentKey key) throws NessieNotFoundException {
+ return getView(BRANCH, key);
+ }
+
+ private IcebergView getView(String ref, ContentKey key) throws
NessieNotFoundException {
+ return
api.getContent().key(key).refName(ref).get().get(key).unwrap(IcebergView.class).get();
+ }
+
+ /** Verify that Nessie always returns the globally-current global-content w/
only DMLs. */
+ @Test
+ public void verifyStateMovesForDML() throws Exception {
+ // 1. initialize view
+ View icebergView = catalog.loadView(VIEW_IDENTIFIER);
+ icebergView
+ .replaceVersion()
+ .withQuery("spark", "some query")
+ .withSchema(SCHEMA)
+ .withDefaultNamespace(VIEW_IDENTIFIER.namespace())
+ .commit();
+
+ // 2. create 2nd branch
+ String testCaseBranch = "verify-global-moving";
+ api.createReference()
+ .sourceRefName(BRANCH)
+ .reference(Branch.of(testCaseBranch, catalog.currentHash()))
+ .create();
+ IcebergView contentInitialMain = getView(BRANCH, KEY);
+ IcebergView contentInitialBranch = getView(testCaseBranch, KEY);
+ View viewInitialMain = catalog.loadView(VIEW_IDENTIFIER);
+
+ // verify view-metadata-location + version-id
+ Assertions.assertThat(contentInitialMain)
+ .as("global-contents + snapshot-id equal on both branches in Nessie")
+ .isEqualTo(contentInitialBranch);
+ Assertions.assertThat(viewInitialMain.currentVersion()).isNotNull();
+
+ // 3. modify view in "main" branch
+
+ icebergView
+ .replaceVersion()
+ .withQuery("trino", "some other query")
+ .withSchema(SCHEMA)
+ .withDefaultNamespace(VIEW_IDENTIFIER.namespace())
+ .commit();
+
+ IcebergView contentsAfter1Main = getView(KEY);
+ IcebergView contentsAfter1Branch = getView(testCaseBranch, KEY);
+ View viewAfter1Main = catalog.loadView(VIEW_IDENTIFIER);
+
+ // --> assert getValue() against both branches returns the updated
metadata-location
+ // verify view-metadata-location
+ Assertions.assertThat(contentInitialMain.getMetadataLocation())
+ .describedAs("metadata-location must change on %s", BRANCH)
+ .isNotEqualTo(contentsAfter1Main.getMetadataLocation());
+ Assertions.assertThat(contentInitialBranch.getMetadataLocation())
+ .describedAs("metadata-location must not change on %s", testCaseBranch)
+ .isEqualTo(contentsAfter1Branch.getMetadataLocation());
+ Assertions.assertThat(contentsAfter1Main)
+ .extracting(IcebergView::getSchemaId)
+ .describedAs("schema ID must be same across branches")
+ .isEqualTo(contentsAfter1Branch.getSchemaId());
+ // verify updates
+ Assertions.assertThat(
+ ((SQLViewRepresentation)
viewAfter1Main.currentVersion().representations().get(0))
+ .dialect())
+ .isEqualTo("trino");
+
+ // 4. modify view in "main" branch again
+
+ icebergView
+ .replaceVersion()
+ .withQuery("flink", "some query")
+ .withSchema(SCHEMA)
+ .withDefaultNamespace(VIEW_IDENTIFIER.namespace())
+ .commit();
+
+ IcebergView contentsAfter2Main = getView(KEY);
+ IcebergView contentsAfter2Branch = getView(testCaseBranch, KEY);
+ View viewAfter2Main = catalog.loadView(VIEW_IDENTIFIER);
+
+ // --> assert getValue() against both branches returns the updated
metadata-location
+ // verify view-metadata-location
+ Assertions.assertThat(contentsAfter2Main.getMetadataLocation())
+ .describedAs("metadata-location must change on %s", BRANCH)
+ .isNotEqualTo(contentsAfter1Main.getMetadataLocation());
+ Assertions.assertThat(contentsAfter2Branch.getMetadataLocation())
+ .describedAs("on-reference-state must not change on %s",
testCaseBranch)
+ .isEqualTo(contentsAfter1Branch.getMetadataLocation());
+ Assertions.assertThat(
+ ((SQLViewRepresentation)
viewAfter2Main.currentVersion().representations().get(0))
+ .dialect())
+ .isEqualTo("flink");
+ }
+
+ @Test
+ public void testUpdate() throws IOException {
+ String viewName = VIEW_IDENTIFIER.name();
+ View icebergView = catalog.loadView(VIEW_IDENTIFIER);
+ // add a column
+ icebergView
+ .replaceVersion()
+ .withQuery("spark", "some query")
+ .withSchema(ALTERED)
+ .withDefaultNamespace(VIEW_IDENTIFIER.namespace())
+ .commit();
+
+ getView(KEY); // sanity, check view exists
+ // check parameters are in expected state
+ String expected = temp.toUri() + DB_NAME + "/" + viewName;
+ Assertions.assertThat(getViewBasePath(viewName)).isEqualTo(expected);
+
+
Assertions.assertThat(metadataVersionFiles(viewLocation)).isNotNull().hasSize(2);
+
+ verifyCommitMetadata();
+ }
+
+ @Test
+ public void testRenameWithTableReference() throws NessieNotFoundException {
+ String renamedViewName = "rename_view_name";
+ TableIdentifier renameViewIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(), renamedViewName);
+
+ ImmutableTableReference fromTableReference =
+ ImmutableTableReference.builder()
+ .reference(catalog.currentRefName())
+ .name(VIEW_IDENTIFIER.name())
+ .build();
+ ImmutableTableReference toTableReference =
+ ImmutableTableReference.builder()
+ .reference(catalog.currentRefName())
+ .name(renameViewIdentifier.name())
+ .build();
+ TableIdentifier fromIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(),
fromTableReference.toString());
+ TableIdentifier toIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(),
toTableReference.toString());
+
+ catalog.renameView(fromIdentifier, toIdentifier);
+ Assertions.assertThat(catalog.viewExists(fromIdentifier)).isFalse();
+ Assertions.assertThat(catalog.viewExists(toIdentifier)).isTrue();
+
+ Assertions.assertThat(catalog.dropView(toIdentifier)).isTrue();
+
+ verifyCommitMetadata();
+ }
+
+ @Test
+ public void testRenameWithTableReferenceInvalidCase() {
+ String renamedViewName = "rename_view_name";
+ TableIdentifier renameViewIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(), renamedViewName);
+
+ ImmutableTableReference fromTableReference =
+ ImmutableTableReference.builder()
+ .reference("Something")
+ .name(VIEW_IDENTIFIER.name())
+ .build();
+ ImmutableTableReference toTableReference =
+ ImmutableTableReference.builder()
+ .reference(catalog.currentRefName())
+ .name(renameViewIdentifier.name())
+ .build();
+ TableIdentifier fromIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(),
fromTableReference.toString());
+ TableIdentifier toIdentifier =
+ TableIdentifier.of(VIEW_IDENTIFIER.namespace(),
toTableReference.toString());
+
+ Assertions.assertThatThrownBy(() -> catalog.renameView(fromIdentifier,
toIdentifier))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(
+ "Cannot rename ICEBERG_VIEW 'view' on reference 'Something' to
'rename_view_name' on reference 'iceberg-view-test' : source and target
references must be the same.");
Review Comment:
```suggestion
"Cannot rename ICEBERG_VIEW 'view' on reference 'Something' to
'rename_view_name' on reference 'iceberg-view-test': source and target
references must be the same.");
```
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
