This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new d8fb60ba33 [#13895] fix(iceberg): report missing views after native
rename (#13896)
d8fb60ba33 is described below
commit d8fb60ba331b87e4ffcda6fc1d96a1afb0cb9f27
Author: Qi Yu <[email protected]>
AuthorDate: Mon Oct 19 14:19:54 2026 +0800
[#13895] fix(iceberg): report missing views after native rename (#13896)
### What changes were proposed in this pull request?
Build Iceberg load-view responses from a checked metadata snapshot. If a
native rename removes the old view between the existence check and
metadata read, throw `NoSuchViewException` with the identifier instead
of passing null metadata to the response builder.
### Why are the changes needed?
Iceberg 1.11 creates a fresh operations instance after checking view
existence. A native rename in between can therefore turn a missing view
into `NullPointerException: metadata` rather than the normal not-found
response.
Fix: #13895
### Does this PR introduce _any_ user-facing change?
A view that disappears during loading produces a normal not-found error
instead of an internal metadata NPE. Successful responses keep the same
metadata and location fields.
### How was this patch tested?
- A real JDBC/SQLite catalog deterministically renames a view between
the two operations instances; the old name reports not-found and the new
name remains readable.
- All non-Docker tests passed on Apache Gravitino `main`
(iceberg-common: 80, catalog-lakehouse-iceberg: 115,
iceberg-rest-server: 466).
- Spotless checks passed.
- Reverting the fix reproduces the original metadata NPE in the
regression; restoring it passes.
- Command: `./gradlew :iceberg:iceberg-common:spotlessCheck
:iceberg:iceberg-common:test :catalogs:catalog-lakehouse-iceberg:test
:iceberg:iceberg-rest-server:test -PskipITs -PskipDockerTests=true` with
`SKIP_DOCKER_TESTS=true`.
---
.../iceberg/common/ops/IcebergCatalogWrapper.java | 27 +++++++++-
.../common/ops/TestIcebergCatalogWrapper.java | 59 ++++++++++++++++++++++
2 files changed, 85 insertions(+), 1 deletion(-)
diff --git
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
index 6ed4f4f965..0304cbf635 100644
---
a/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
+++
b/iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java
@@ -47,6 +47,7 @@ import org.apache.iceberg.catalog.SupportsNamespaces;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.catalog.ViewCatalog;
import org.apache.iceberg.exceptions.NoSuchTableException;
+import org.apache.iceberg.exceptions.NoSuchViewException;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.io.ResolvingFileIO;
import org.apache.iceberg.jdbc.JdbcCatalogWithMetadataLocationSupport;
@@ -63,11 +64,15 @@ import
org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest;
import org.apache.iceberg.rest.requests.UpdateTableRequest;
import org.apache.iceberg.rest.responses.CreateNamespaceResponse;
import org.apache.iceberg.rest.responses.GetNamespaceResponse;
+import org.apache.iceberg.rest.responses.ImmutableLoadViewResponse;
import org.apache.iceberg.rest.responses.ListNamespacesResponse;
import org.apache.iceberg.rest.responses.ListTablesResponse;
import org.apache.iceberg.rest.responses.LoadTableResponse;
import org.apache.iceberg.rest.responses.LoadViewResponse;
import org.apache.iceberg.rest.responses.UpdateNamespacePropertiesResponse;
+import org.apache.iceberg.view.BaseView;
+import org.apache.iceberg.view.View;
+import org.apache.iceberg.view.ViewMetadata;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -375,8 +380,28 @@ public class IcebergCatalogWrapper implements
AutoCloseable {
return CatalogHandlers.updateView(getViewCatalog(), viewIdentifier,
request);
}
+ /**
+ * Loads a view response from one metadata snapshot.
+ *
+ * @param viewIdentifier the view to load
+ * @return the view metadata and its location
+ * @throws NoSuchViewException if the view does not exist or disappears
during loading
+ */
public LoadViewResponse loadView(TableIdentifier viewIdentifier) {
- return CatalogHandlers.loadView(getViewCatalog(), viewIdentifier);
+ View view = getViewCatalog().loadView(viewIdentifier);
+ Preconditions.checkState(
+ view instanceof BaseView, "Cannot wrap catalog that does not produce
BaseView");
+ ViewMetadata metadata = ((BaseView) view).operations().current();
+ if (metadata == null) {
+ throw new NoSuchViewException("View does not exist: %s", viewIdentifier);
+ }
+ // CatalogHandlers' private viewResponse helper assumes non-null metadata.
A native rename
+ // can remove the view after loadView's existence check, so build the same
response from the
+ // checked snapshot rather than loading it again through the handler.
+ return ImmutableLoadViewResponse.builder()
+ .metadata(metadata)
+ .metadataLocation(metadata.metadataFileLocation())
+ .build();
}
public void dropView(TableIdentifier viewIdentifier) {
diff --git
a/iceberg/iceberg-common/src/test/java/org/apache/gravitino/iceberg/common/ops/TestIcebergCatalogWrapper.java
b/iceberg/iceberg-common/src/test/java/org/apache/gravitino/iceberg/common/ops/TestIcebergCatalogWrapper.java
index 48941d7f8f..9b87546e25 100644
---
a/iceberg/iceberg-common/src/test/java/org/apache/gravitino/iceberg/common/ops/TestIcebergCatalogWrapper.java
+++
b/iceberg/iceberg-common/src/test/java/org/apache/gravitino/iceberg/common/ops/TestIcebergCatalogWrapper.java
@@ -33,6 +33,7 @@ import org.apache.gravitino.iceberg.common.IcebergConfig;
import org.apache.gravitino.iceberg.common.cache.SupportsMetadataLocation;
import org.apache.gravitino.iceberg.common.cache.TableMetadataCache;
import org.apache.gravitino.iceberg.common.utils.IcebergCatalogUtil;
+import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.TableMetadata;
@@ -40,14 +41,72 @@ import org.apache.iceberg.TableMetadataParser;
import org.apache.iceberg.catalog.Catalog;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.NoSuchViewException;
+import org.apache.iceberg.jdbc.JdbcCatalog;
import org.apache.iceberg.rest.requests.CreateNamespaceRequest;
+import org.apache.iceberg.rest.responses.LoadViewResponse;
import org.apache.iceberg.types.Types;
+import org.apache.iceberg.view.ViewOperations;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
+import org.mockito.Mockito;
public class TestIcebergCatalogWrapper {
+ @Test
+ void testLoadViewReportsMissingAfterNativeRename(@TempDir Path warehouseDir)
throws Exception {
+ Namespace namespace = Namespace.of("views");
+ TableIdentifier oldName = TableIdentifier.of(namespace, "old_name");
+ TableIdentifier newName = TableIdentifier.of(namespace, "new_name");
+ AtomicBoolean renameDuringLoad = new AtomicBoolean();
+ AtomicInteger operationLoads = new AtomicInteger();
+ try (JdbcCatalog catalog =
+ new JdbcCatalog() {
+ @Override
+ protected ViewOperations newViewOps(TableIdentifier identifier) {
+ // BaseMetastoreViewCatalog checks one operations instance, then
returns a BaseView
+ // backed by another. Rename between those two instances, without
timing or threads.
+ if (renameDuringLoad.get()
+ && identifier.equals(oldName)
+ && operationLoads.incrementAndGet() == 2) {
+ renameView(oldName, newName);
+ }
+ return super.newViewOps(identifier);
+ }
+ }) {
+ catalog.initialize(
+ "rename-test",
+ Map.of(
+ CatalogProperties.URI,
+ "jdbc:sqlite:" + warehouseDir.resolve("catalog.db"),
+ CatalogProperties.WAREHOUSE_LOCATION,
+ warehouseDir.toUri().toString(),
+ "jdbc.schema-version",
+ "V1"));
+ catalog.createNamespace(namespace);
+ catalog
+ .buildView(oldName)
+ .withSchema(new Schema(Types.NestedField.required(1, "id",
Types.IntegerType.get())))
+ .withDefaultNamespace(namespace)
+ .withQuery("spark", "SELECT 1 AS id")
+ .create();
+ IcebergCatalogWrapper wrapper =
+ Mockito.spy(new IcebergCatalogWrapper(new
IcebergConfig(unreachableConfig())));
+ Mockito.doReturn(catalog).when(wrapper).getCatalog();
+ renameDuringLoad.set(true);
+
+ NoSuchViewException failure =
+ Assertions.assertThrows(NoSuchViewException.class, () ->
wrapper.loadView(oldName));
+ Assertions.assertTrue(failure.getMessage().contains(oldName.toString()));
+ Assertions.assertFalse(catalog.viewExists(oldName));
+ LoadViewResponse response = wrapper.loadView(newName);
+ Assertions.assertNotNull(response.metadata());
+ Assertions.assertEquals(
+ response.metadata().metadataFileLocation(),
response.metadataLocation());
+ }
+ }
+
@Test
public void testCatalogShouldBeLazyLoaded() {
IcebergCatalogWrapper wrapper =
