Skip to content

[fix](iceberg) Allow disabling REST catalog view operations #62986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xylaaaaa wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[fix](iceberg) Allow disabling REST catalog view operations #62986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public List<String> listTableNames(String dbName) {
// IcebergMetadataOps handles listTableNames and listViewNames separately.
// listTableNames should only focus on the table type,
// but in reality, Iceberg's return includes views. Therefore, we added a filter to exclude views.
if (catalog instanceof ViewCatalog) {
if (isViewCatalogEnabled()) {
views = ((ViewCatalog) catalog).listViews(getNamespace(dbName))
.stream().map(TableIdentifier::name).collect(Collectors.toList());
} else {
Expand Down Expand Up @@ -1126,7 +1126,7 @@ public Table loadTable(String dbName, String tblName) {

@Override
public boolean viewExists(String remoteDbName, String remoteViewName) {
if (!(catalog instanceof ViewCatalog)) {
if (!isViewCatalogEnabled()) {
return false;
}
try {
Expand All @@ -1140,7 +1140,7 @@ public boolean viewExists(String remoteDbName, String remoteViewName) {

@Override
public Object loadView(String dbName, String tblName) {
if (!(catalog instanceof ViewCatalog)) {
if (!isViewCatalogEnabled()) {
return null;
}
try {
Expand All @@ -1154,7 +1154,7 @@ public Object loadView(String dbName, String tblName) {

@Override
public List<String> listViewNames(String db) {
if (!(catalog instanceof ViewCatalog)) {
if (!isViewCatalogEnabled()) {
return Collections.emptyList();
}
try {
Expand Down Expand Up @@ -1192,12 +1192,25 @@ private Namespace getNamespace() {
return externalCatalogName.map(Namespace::of).orElseGet(() -> Namespace.empty());
}

private boolean isViewCatalogEnabled() {
if (!(catalog instanceof ViewCatalog)) {
return false;
}
if (dorisCatalog instanceof IcebergRestExternalCatalog) {
MetastoreProperties metaProps = dorisCatalog.getCatalogProperty().getMetastoreProperties();
if (metaProps instanceof IcebergRestProperties) {
return ((IcebergRestProperties) metaProps).isIcebergRestViewEnabled();
}
}
return true;
}

public ThreadPoolExecutor getThreadPoolWithPreAuth() {
return dorisCatalog.getThreadPoolWithPreAuth();
}

private void performDropView(String remoteDbName, String remoteViewName) throws DdlException {
if (!(catalog instanceof ViewCatalog)) {
if (!isViewCatalogEnabled()) {
throw new DdlException("Drop Iceberg view is not supported with not view catalog.");
}
Comment on lines +1213 to 1215
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DdlException message here becomes misleading when view ops are disabled via iceberg.rest.view-enabled=false: isViewCatalogEnabled() can return false even though catalog is a ViewCatalog. Consider updating the message (or branching) to indicate that view operations are disabled by configuration, not that the catalog lacks ViewCatalog support.

Copilot uses AI. Check for mistakes.
ViewCatalog viewCatalog = (ViewCatalog) catalog;
Expand Down
9 changes: 9 additions & 0 deletions ...e/src/main/java/org/apache/doris/datasource/property/metastore/IcebergRestProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ public class IcebergRestProperties extends AbstractIcebergProperties {
description = "Enable nested namespace for the iceberg rest catalog service.")
private String icebergRestNestedNamespaceEnabled = "false";

@ConnectorProperty(names = {"iceberg.rest.view-enabled"},
required = false,
description = "Enable view operations for the iceberg rest catalog service.")
private String icebergRestViewEnabled = "true";

@ConnectorProperty(names = {"iceberg.rest.case-insensitive-name-matching"},
required = false,
supported = false,
Expand Down Expand Up @@ -317,6 +322,10 @@ public boolean isIcebergRestNestedNamespaceEnabled() {
return Boolean.parseBoolean(icebergRestNestedNamespaceEnabled);
}

public boolean isIcebergRestViewEnabled() {
return Boolean.parseBoolean(icebergRestViewEnabled);
}

public enum Security {
NONE,
OAUTH2,
Expand Down
69 changes: 69 additions & 0 deletions fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,23 @@

package org.apache.doris.datasource.iceberg;

import org.apache.doris.common.security.authentication.ExecutionAuthenticator;
import org.apache.doris.datasource.CatalogProperty;

import org.apache.iceberg.catalog.Catalog;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.SupportsNamespaces;
import org.apache.iceberg.catalog.TableIdentifier;
import org.apache.iceberg.catalog.ViewCatalog;
import org.junit.Assert;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;

public class IcebergMetadataOpTest {
Expand All @@ -45,4 +58,60 @@ public void testGetNamespaces() {
ns = IcebergMetadataOps.getNamespace(Optional.empty(), "");
Assert.assertEquals(0, ns.length());
}

@Test
public void testListTableNamesSkipsViewsWhenRestViewDisabled() {
IcebergRestExternalCatalog dorisCatalog = Mockito.mock(IcebergRestExternalCatalog.class);
Catalog icebergCatalog = Mockito.mock(Catalog.class,
Mockito.withSettings().extraInterfaces(SupportsNamespaces.class, ViewCatalog.class));

Map<String, String> props = new HashMap<>();
props.put("type", "iceberg");
props.put("iceberg.catalog.type", "rest");
props.put("iceberg.rest.uri", "http://localhost:8181");
props.put("iceberg.rest.view-enabled", "false");

Mockito.when(dorisCatalog.getExecutionAuthenticator()).thenReturn(new ExecutionAuthenticator() {
});
Mockito.when(dorisCatalog.getProperties()).thenReturn(Collections.emptyMap());
Mockito.when(dorisCatalog.getCatalogProperty()).thenReturn(new CatalogProperty(null, props));

Namespace namespace = Namespace.of("PUBLIC");
TableIdentifier table = TableIdentifier.of(namespace, "DORIS_HORIZON_T");
Mockito.when(icebergCatalog.listTables(namespace)).thenReturn(Collections.singletonList(table));

IcebergMetadataOps ops = new IcebergMetadataOps(dorisCatalog, icebergCatalog);
List<String> tableNames = ops.listTableNames("PUBLIC");

Assert.assertEquals(Collections.singletonList("DORIS_HORIZON_T"), tableNames);
Mockito.verify((ViewCatalog) icebergCatalog, Mockito.never()).listViews(Mockito.any());
}

@Test
public void testListTableNamesFiltersViewsWhenRestViewEnabled() {
IcebergRestExternalCatalog dorisCatalog = Mockito.mock(IcebergRestExternalCatalog.class);
Catalog icebergCatalog = Mockito.mock(Catalog.class,
Mockito.withSettings().extraInterfaces(SupportsNamespaces.class, ViewCatalog.class));

Map<String, String> props = new HashMap<>();
props.put("type", "iceberg");
props.put("iceberg.catalog.type", "rest");
props.put("iceberg.rest.uri", "http://localhost:8181");

Mockito.when(dorisCatalog.getExecutionAuthenticator()).thenReturn(new ExecutionAuthenticator() {
});
Mockito.when(dorisCatalog.getProperties()).thenReturn(Collections.emptyMap());
Mockito.when(dorisCatalog.getCatalogProperty()).thenReturn(new CatalogProperty(null, props));

Namespace namespace = Namespace.of("PUBLIC");
TableIdentifier table = TableIdentifier.of(namespace, "DORIS_HORIZON_T");
TableIdentifier view = TableIdentifier.of(namespace, "DORIS_HORIZON_V");
Mockito.when(icebergCatalog.listTables(namespace)).thenReturn(Arrays.asList(table, view));
Mockito.when(((ViewCatalog) icebergCatalog).listViews(namespace)).thenReturn(Collections.singletonList(view));

IcebergMetadataOps ops = new IcebergMetadataOps(dorisCatalog, icebergCatalog);
List<String> tableNames = ops.listTableNames("PUBLIC");

Assert.assertEquals(Collections.singletonList("DORIS_HORIZON_T"), tableNames);
}
}
17 changes: 17 additions & 0 deletions ...c/test/java/org/apache/doris/datasource/property/metastore/IcebergRestPropertiesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,23 @@ public void testVendedCredentialsDisabled() {
Assertions.assertFalse(catalogProps.containsKey("header.X-Iceberg-Access-Delegation"));
}

@Test
public void testRestViewEnabled() {
Map<String, String> props = new HashMap<>();
props.put("iceberg.rest.uri", "http://localhost:8080");

IcebergRestProperties defaultProps = new IcebergRestProperties(props);
defaultProps.initNormalizeAndCheckProps();
Assertions.assertTrue(defaultProps.isIcebergRestViewEnabled());

props.put("iceberg.rest.view-enabled", "false");
IcebergRestProperties disabledProps = new IcebergRestProperties(props);
disabledProps.initNormalizeAndCheckProps();
Assertions.assertFalse(disabledProps.isIcebergRestViewEnabled());
Assertions.assertFalse(disabledProps.getIcebergRestCatalogProperties()
.containsKey("iceberg.rest.view-enabled"));
}

@Test
public void testOAuth2CredentialFlow() {
Map<String, String> props = new HashMap<>();
Expand Down
Loading