Gabriel39 commented on code in PR #67545: URL: https://github.com/apache/doris/pull/67545#discussion_r3942843747
########## fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/dlf/HiveCompatibleCatalog.java: ########## @@ -0,0 +1,166 @@ +// 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.doris.connector.iceberg.dlf; + +import org.apache.hadoop.conf.Configurable; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.iceberg.BaseMetastoreCatalog; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; +import org.apache.iceberg.ClientPool; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SupportsNamespaces; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.NamespaceNotEmptyException; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; +import org.apache.iceberg.exceptions.NoSuchTableException; +import org.apache.iceberg.hadoop.HadoopFileIO; +import org.apache.iceberg.io.FileIO; +import shade.doris.hive.org.apache.thrift.TException; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +/** Base catalog for Hive-compatible metastores that need a custom client pool. */ +public abstract class HiveCompatibleCatalog extends BaseMetastoreCatalog implements SupportsNamespaces, Configurable { + + protected Configuration conf; + protected ClientPool<IMetaStoreClient, TException> clients; + protected FileIO fileIO; + protected String catalogName; + + public void initialize(String name, FileIO fileIO, ClientPool<IMetaStoreClient, TException> clients) { Review Comment: Added synchronized, idempotent catalog cleanup that closes FileIO, invalidates and synchronously cleans the DLF client-pool cache, and still invokes super.close() while preserving suppressed failures. Tests cover repeated close and both file-IO-only and client-only partial initialization. ########## fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/dlf/HiveCompatibleCatalog.java: ########## @@ -0,0 +1,274 @@ +// 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.doris.connector.iceberg.dlf; + +import org.apache.hadoop.conf.Configurable; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.Database; +import org.apache.hadoop.hive.metastore.api.NoSuchObjectException; +import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.UnknownDBException; +import org.apache.iceberg.BaseMetastoreCatalog; +import org.apache.iceberg.BaseMetastoreTableOperations; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; +import org.apache.iceberg.ClientPool; +import org.apache.iceberg.Schema; +import org.apache.iceberg.catalog.Catalog.TableBuilder; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SupportsNamespaces; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.NamespaceNotEmptyException; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; +import org.apache.iceberg.hadoop.HadoopFileIO; +import org.apache.iceberg.hive.HiveCatalog; +import org.apache.iceberg.io.FileIO; +import shade.doris.hive.org.apache.thrift.TException; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +/** Base catalog for Hive-compatible metastores that need a custom client pool. */ +public abstract class HiveCompatibleCatalog extends BaseMetastoreCatalog implements SupportsNamespaces, Configurable { + + protected Configuration conf; + protected ClientPool<IMetaStoreClient, TException> clients; + protected FileIO fileIO; + protected String catalogName; + private boolean listAllTables; + private boolean closed; + + public void initialize(String name, FileIO fileIO, ClientPool<IMetaStoreClient, TException> clients) { + initialize(name, fileIO, clients, Map.of()); + } + + public void initialize(String name, FileIO fileIO, ClientPool<IMetaStoreClient, TException> clients, Review Comment: Fixed. HiveCompatibleCatalog now preserves the full immutable catalog property map and exposes it through properties(), allowing BaseMetastoreCatalog to initialize and close the configured metrics reporter. Added a lifecycle test that verifies initialization receives the retained properties and close is invoked. ########## fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/dlf/HiveCompatibleCatalog.java: ########## @@ -0,0 +1,274 @@ +// 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.doris.connector.iceberg.dlf; + +import org.apache.hadoop.conf.Configurable; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.Database; +import org.apache.hadoop.hive.metastore.api.NoSuchObjectException; +import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.UnknownDBException; +import org.apache.iceberg.BaseMetastoreCatalog; +import org.apache.iceberg.BaseMetastoreTableOperations; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; +import org.apache.iceberg.ClientPool; +import org.apache.iceberg.Schema; +import org.apache.iceberg.catalog.Catalog.TableBuilder; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SupportsNamespaces; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.NamespaceNotEmptyException; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; +import org.apache.iceberg.hadoop.HadoopFileIO; +import org.apache.iceberg.hive.HiveCatalog; +import org.apache.iceberg.io.FileIO; +import shade.doris.hive.org.apache.thrift.TException; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +/** Base catalog for Hive-compatible metastores that need a custom client pool. */ +public abstract class HiveCompatibleCatalog extends BaseMetastoreCatalog implements SupportsNamespaces, Configurable { + + protected Configuration conf; + protected ClientPool<IMetaStoreClient, TException> clients; + protected FileIO fileIO; + protected String catalogName; + private boolean listAllTables; + private boolean closed; + + public void initialize(String name, FileIO fileIO, ClientPool<IMetaStoreClient, TException> clients) { + initialize(name, fileIO, clients, Map.of()); + } + + public void initialize(String name, FileIO fileIO, ClientPool<IMetaStoreClient, TException> clients, + Map<String, String> properties) { + this.catalogName = name; + this.fileIO = fileIO; + this.clients = clients; + this.listAllTables = Boolean.parseBoolean(properties.getOrDefault( + HiveCatalog.LIST_ALL_TABLES, HiveCatalog.LIST_ALL_TABLES_DEFAULT)); + } + + protected FileIO initializeFileIO(Map<String, String> properties, Configuration hadoopConf) { + String fileIOImpl = properties.get(CatalogProperties.FILE_IO_IMPL); + if (fileIOImpl == null) { + FileIO io = new HadoopFileIO(hadoopConf); + io.initialize(properties); + return io; + } + return CatalogUtil.loadFileIO(fileIOImpl, properties, hadoopConf); + } + + @Override + protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) { + return null; + } + + @Override + protected boolean isValidIdentifier(TableIdentifier tableIdentifier) { + return tableIdentifier.namespace().levels().length == 1; + } + + protected boolean isValidNamespace(Namespace namespace) { + return namespace.levels().length == 1; + } + + @Override + public List<TableIdentifier> listTables(Namespace namespace) { + if (!isValidNamespace(namespace)) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + String dbName = namespace.level(0); + try { + List<String> tableNames = clients.run(client -> client.getAllTables(dbName)); + if (listAllTables) { + return tableNames.stream() + .map(table -> TableIdentifier.of(dbName, table)) + .collect(Collectors.toList()); + } + // DLF namespaces are format-shared; publishing non-Iceberg names creates unusable Doris tables. + List<Table> tables = clients.run(client -> client.getTableObjectsByName(dbName, tableNames)); + return tables.stream() + .filter(table -> table.getParameters() != null + && BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE.equalsIgnoreCase( + table.getParameters().get(BaseMetastoreTableOperations.TABLE_TYPE_PROP))) + .map(Table::getTableName) + .map(table -> TableIdentifier.of(dbName, table)) + .collect(Collectors.toList()); + } catch (UnknownDBException e) { + throw new NoSuchNamespaceException(e, "Namespace does not exist: %s", namespace); + } catch (TException e) { + throw new RuntimeException("Failed to list tables under namespace " + namespace, e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Interrupted in call to listTables", e); + } + } + + @Override + public TableBuilder buildTable(TableIdentifier identifier, Schema schema) { + // DLF metadata writes were never supported; reject before BaseMetastoreCatalog builds a null location. + throw new UnsupportedOperationException("Cannot create table " + identifier + ": not supported"); + } + + @Override + public boolean dropTable(TableIdentifier tableIdentifier, boolean purge) { + throw new UnsupportedOperationException("Cannot drop table " + tableIdentifier + ": not supported"); + } + + @Override + public void renameTable(TableIdentifier source, TableIdentifier target) { + throw new UnsupportedOperationException("Cannot rename table " + source + ": not supported"); + } + + @Override + public void createNamespace(Namespace namespace, Map<String, String> properties) { + throw new UnsupportedOperationException("Cannot create namespace " + namespace + ": not supported"); + } + + @Override + public List<Namespace> listNamespaces(Namespace namespace) throws NoSuchNamespaceException { + if (!isValidNamespace(namespace) && !namespace.isEmpty()) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + if (!namespace.isEmpty()) { Review Comment: Fixed by rejecting external_catalog.name for Iceberg DLF during CREATE/ALTER validation, while keeping replay binding non-failing. This prevents both listing and direct table lookup from entering an inconsistent namespace mapping. Added unit and regression coverage for the rejection. ########## fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java: ########## @@ -465,11 +466,32 @@ private Catalog createCatalog() { hmsAuth, storageHadoopConfig, "Failed to create Paimon catalog with HMS metastore"); } + case PaimonCatalogProperties.DLF: { + // Legacy DLF catalogs often expose OSS only through dlf.* aliases and an oss:// warehouse. + // Check the resolved storage bindings here so those catalogs remain valid while non-OSS + // backends cannot be passed to Paimon's DLF Hive catalog. + if (!hasDlfCompatibleStorage(storage().getStorageProperties())) { + throw new IllegalStateException("Paimon DLF metastore requires OSS storage properties."); + } + DlfMetaStoreProperties dlf = (DlfMetaStoreProperties) + MetaStoreProviders.bind(catalogProps.getRaw(), storageHadoopConfig); + Map<String, String> dlfConf = new HashMap<>(dlf.toDlfCatalogConf()); + dlfConf.put(PaimonCatalogFactory.DLF_CLIENT_POOL_IDENTITY, + PaimonCatalogFactory.dlfClientPoolIdentity(dlfConf)); + HiveConf hc = PaimonCatalogFactory.assembleHiveConf(null, dlfConf); + return createCatalogFromContext(CatalogContext.create(options, hc), flavor, + "Failed to create Paimon catalog with DLF metastore"); + } default: throw new IllegalArgumentException("Unknown paimon.catalog.type value: " + flavor); } } + static boolean hasDlfCompatibleStorage(List<StorageProperties> storageProperties) { + return storageProperties.stream().anyMatch(storage -> "OSS".equals(storage.providerName()) Review Comment: Fixed. DLF-to-OSS-DLS conversion now uses the same case-insensitive parsed DLF endpoint pattern as region extraction. Added a mixed-case DLF-VPC hostname test. ########## fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/dlf/HiveCompatibleCatalog.java: ########## @@ -0,0 +1,274 @@ +// 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.doris.connector.iceberg.dlf; + +import org.apache.hadoop.conf.Configurable; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.Database; +import org.apache.hadoop.hive.metastore.api.NoSuchObjectException; +import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.UnknownDBException; +import org.apache.iceberg.BaseMetastoreCatalog; +import org.apache.iceberg.BaseMetastoreTableOperations; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; +import org.apache.iceberg.ClientPool; +import org.apache.iceberg.Schema; +import org.apache.iceberg.catalog.Catalog.TableBuilder; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.SupportsNamespaces; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.NamespaceNotEmptyException; +import org.apache.iceberg.exceptions.NoSuchNamespaceException; +import org.apache.iceberg.hadoop.HadoopFileIO; +import org.apache.iceberg.hive.HiveCatalog; +import org.apache.iceberg.io.FileIO; +import shade.doris.hive.org.apache.thrift.TException; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +/** Base catalog for Hive-compatible metastores that need a custom client pool. */ +public abstract class HiveCompatibleCatalog extends BaseMetastoreCatalog implements SupportsNamespaces, Configurable { + + protected Configuration conf; + protected ClientPool<IMetaStoreClient, TException> clients; + protected FileIO fileIO; + protected String catalogName; + private boolean listAllTables; + private boolean closed; + + public void initialize(String name, FileIO fileIO, ClientPool<IMetaStoreClient, TException> clients) { + initialize(name, fileIO, clients, Map.of()); + } + + public void initialize(String name, FileIO fileIO, ClientPool<IMetaStoreClient, TException> clients, + Map<String, String> properties) { + this.catalogName = name; + this.fileIO = fileIO; + this.clients = clients; + this.listAllTables = Boolean.parseBoolean(properties.getOrDefault( + HiveCatalog.LIST_ALL_TABLES, HiveCatalog.LIST_ALL_TABLES_DEFAULT)); + } + + protected FileIO initializeFileIO(Map<String, String> properties, Configuration hadoopConf) { + String fileIOImpl = properties.get(CatalogProperties.FILE_IO_IMPL); + if (fileIOImpl == null) { + FileIO io = new HadoopFileIO(hadoopConf); + io.initialize(properties); + return io; + } + return CatalogUtil.loadFileIO(fileIOImpl, properties, hadoopConf); + } + + @Override + protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) { + return null; + } + + @Override + protected boolean isValidIdentifier(TableIdentifier tableIdentifier) { + return tableIdentifier.namespace().levels().length == 1; + } + + protected boolean isValidNamespace(Namespace namespace) { + return namespace.levels().length == 1; + } + + @Override + public List<TableIdentifier> listTables(Namespace namespace) { + if (!isValidNamespace(namespace)) { + throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace); + } + String dbName = namespace.level(0); + try { + List<String> tableNames = clients.run(client -> client.getAllTables(dbName)); + if (listAllTables) { + return tableNames.stream() + .map(table -> TableIdentifier.of(dbName, table)) + .collect(Collectors.toList()); + } + // DLF namespaces are format-shared; publishing non-Iceberg names creates unusable Doris tables. + List<Table> tables = clients.run(client -> client.getTableObjectsByName(dbName, tableNames)); Review Comment: Fixed. DLF full-table metadata requests are now capped at 100 names per BatchGet call, and each response is filtered immediately so full table descriptors do not accumulate across batches. Added a 205-table test that verifies 100/100/5 request sizes. ########## fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalog.java: ########## @@ -522,6 +522,14 @@ public void overlayMetaCacheConfig(Map<String, String> metaCacheProperties) { */ @Override public boolean createTable(CreateTableInfo createTableInfo) throws UserException { + Map<String, String> properties = getProperties(); + try { + // Unsupported configuration-specific DDL must fail before initialization can touch a remote service. + ConnectorFactory.findProvider(getType(), properties) + .ifPresent(provider -> provider.validateCreateTable(properties)); Review Comment: Fixed. Non-IF CTAS performs configuration-only catalog preflight before the target lookup, while IF NOT EXISTS first allows the catalog to return the required existing-target no-op and validates only an absent target. Tests cover non-IF CTAS ordering, IF NOT EXISTS CTAS short-circuiting, and the plain existing-target no-op. ########## fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java: ########## @@ -465,11 +469,32 @@ private Catalog createCatalog() { hmsAuth, storageHadoopConfig, "Failed to create Paimon catalog with HMS metastore"); } + case PaimonCatalogProperties.DLF: { + // Legacy DLF catalogs often expose OSS only through dlf.* aliases and an oss:// warehouse. + // Check the resolved storage bindings here so those catalogs remain valid while non-OSS + // backends cannot be passed to Paimon's DLF Hive catalog. + if (!hasDlfCompatibleStorage(storage().getStorageProperties())) { + throw new IllegalStateException("Paimon DLF metastore requires OSS storage properties."); + } + DlfMetaStoreProperties dlf = (DlfMetaStoreProperties) + MetaStoreProviders.bind(catalogProps.getRaw(), storageHadoopConfig); + Map<String, String> dlfConf = new HashMap<>(dlf.toDlfCatalogConf()); + dlfConf.put(PaimonCatalogFactory.DLF_CLIENT_POOL_IDENTITY, + PaimonCatalogFactory.dlfClientPoolIdentity(dlfConf)); + HiveConf hc = PaimonCatalogFactory.assembleHiveConf(null, dlfConf); + return createCatalogFromContext(CatalogContext.create(options, hc), flavor, + "Failed to create Paimon catalog with DLF metastore"); + } default: throw new IllegalArgumentException("Unknown paimon.catalog.type value: " + flavor); } } + static boolean hasDlfCompatibleStorage(List<StorageProperties> storageProperties) { + return storageProperties.stream().anyMatch(storage -> "OSS".equals(storage.providerName()) Review Comment: Fixed in 5993f0e488. Plain oss:// paths now fall back to the catalog OSS_HDFS adapter when native OSS is absent. The adapter embeds its configured oss-dls endpoint during normalization, so its Jindo identity survives the connector string boundary and BE keeps FILE_HDFS routing. Added real StorageAdapter.ofAll coverage for both data and deletion-vector paths, plus direct OSS-HDFS normalization coverage. The focused unit tests, full FE package/Checkstyle build, and external_table_p0/test_dlf_catalog regression suite pass locally. ########## fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java: ########## @@ -465,11 +466,32 @@ private Catalog createCatalog() { hmsAuth, storageHadoopConfig, "Failed to create Paimon catalog with HMS metastore"); } + case PaimonCatalogProperties.DLF: { Review Comment: Fixed. Paimon DLF test_connection now forces an authenticated, plugin-TCCL metadata listing and then probes both FE and BE storage connectivity. Added focused unit coverage for all three legs and a regression case proving an unreachable Paimon DLF catalog is rejected. ########## fe/fe-connector/fe-connector-metastore-spi/src/main/java/org/apache/doris/connector/metastore/spi/AbstractDlfMetaStoreProperties.java: ########## @@ -0,0 +1,126 @@ +// 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.doris.connector.metastore.spi; + +import org.apache.doris.connector.metastore.DlfMetaStoreProperties; +import org.apache.doris.foundation.property.ConnectorProperty; + +import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.StringUtils; + +import java.util.LinkedHashMap; +import java.util.Map; + +/** Shared Aliyun DLF property binding and neutral catalog configuration. */ +public abstract class AbstractDlfMetaStoreProperties extends AbstractMetaStoreProperties + implements DlfMetaStoreProperties { + + // These aliases must stay aligned with both OSS binders so one credential set reaches metadata and storage. + @ConnectorProperty(names = {"dlf.access_key", "dlf.catalog.accessKeyId"}, required = false, sensitive = true, + description = "DLF access key id.") + private String accessKey = ""; + + @ConnectorProperty(names = {"dlf.secret_key", "dlf.catalog.secret_key", "dlf.catalog.accessKeySecret"}, + required = false, sensitive = true, + description = "DLF access key secret.") + private String secretKey = ""; + + @ConnectorProperty(names = {"dlf.session_token", "dlf.catalog.sessionToken", "dlf.catalog.securityToken"}, Review Comment: Fixed. dlf.catalog.securityToken is now part of the provider-independent, case-insensitive sensitive-key inventory. Added a direct DatasourcePrintableMap test proving the raw token is masked without provider registration. ########## fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalog.java: ########## @@ -522,6 +522,14 @@ public void overlayMetaCacheConfig(Map<String, String> metaCacheProperties) { */ @Override public boolean createTable(CreateTableInfo createTableInfo) throws UserException { + Map<String, String> properties = getProperties(); + try { + // Unsupported configuration-specific DDL must fail before initialization can touch a remote service. + ConnectorFactory.findProvider(getType(), properties) Review Comment: Fixed. CREATE TABLE provider validation now routes through ConnectorPluginManager, which pins the provider defining classloader around both supports() and validateCreateTable() and restores the caller TCCL. The temporary directory-plugin fixture now resolves a plugin-local helper from this callback and verifies restoration. ########## fe/fe-filesystem/fe-filesystem-oss-hdfs/src/main/java/org/apache/doris/filesystem/hdfs/properties/OssHdfsProperties.java: ########## @@ -137,7 +142,30 @@ public String validateAndNormalizeUri(String uri) { if (!uriObj.getScheme().equalsIgnoreCase("oss")) { throw new IllegalArgumentException("The uri scheme is not oss."); } - return uriObj.toString(); + String authority = uriObj.getRawAuthority(); + if (StringUtils.isBlank(authority) + || authority.toLowerCase(Locale.ROOT).endsWith(OSS_HDFS_ENDPOINT_SUFFIX)) { + return uriObj.toString(); + } + + // The connector SPI carries only the normalized URI, not the selected adapter. Embedding + // the configured endpoint keeps plain bucket paths identifiable as Jindo/HDFS downstream. + String endpointHost = extractEndpointHost(endpoint); + String normalizedUri = uriObj.toString(); + int authorityStart = uriObj.getScheme().length() + 3; + int authorityEnd = authorityStart + authority.length(); + return normalizedUri.substring(0, authorityStart) + + authority + "." + endpointHost + + normalizedUri.substring(authorityEnd); Review Comment: Fixed. OSS-HDFS normalization now extracts the bucket from native OSS-qualified authorities, including internal endpoints, and rebuilds it with the configured DLS endpoint instead of appending. Added data, deletion-vector, write file-type, and qualified-authority coverage. ########## fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java: ########## @@ -503,7 +516,12 @@ ConnectorTestResult probeStorageFromBackend(String location) { * (Iceberg {@code warehouse} or Polaris {@code default-base-location}). */ private String resolveS3TestLocation(String catalogType) { - String location = toS3Location(properties.get(CatalogProperties.WAREHOUSE_LOCATION)); + String warehouse = properties.get(CatalogProperties.WAREHOUSE_LOCATION); + if (IcebergCatalogProperties.TYPE_DLF.equalsIgnoreCase(catalogType) + && warehouse != null && warehouse.trim().toLowerCase(Locale.ROOT).startsWith("oss://")) { + warehouse = "s3://" + warehouse.trim().substring("oss://".length()); + } Review Comment: Fixed. The DLF storage probe now runs the warehouse through the selected storage binding before deriving the S3 probe location, so FE and BE receive s3://bucket/... for qualified OSS warehouses. Added a connectivity location-resolution test. ########## fe/fe-filesystem/fe-filesystem-oss-hdfs/src/main/java/org/apache/doris/filesystem/hdfs/properties/OssHdfsProperties.java: ########## @@ -137,7 +142,30 @@ public String validateAndNormalizeUri(String uri) { if (!uriObj.getScheme().equalsIgnoreCase("oss")) { throw new IllegalArgumentException("The uri scheme is not oss."); } - return uriObj.toString(); + String authority = uriObj.getRawAuthority(); + if (StringUtils.isBlank(authority) + || authority.toLowerCase(Locale.ROOT).endsWith(OSS_HDFS_ENDPOINT_SUFFIX)) { + return uriObj.toString(); Review Comment: Fixed. OSS-HDFS-qualified authorities are now canonicalized through the configured lowercase DLS endpoint, so downstream dispatch preserves FILE_HDFS for mixed-case data and deletion-vector URIs. Added end-to-end normalization and backend file-type coverage. ########## fe/fe-connector/fe-connector-metastore-spi/src/main/java/org/apache/doris/connector/metastore/spi/AbstractDlfMetaStoreProperties.java: ########## @@ -0,0 +1,126 @@ +// 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.doris.connector.metastore.spi; + +import org.apache.doris.connector.metastore.DlfMetaStoreProperties; +import org.apache.doris.foundation.property.ConnectorProperty; + +import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.StringUtils; + +import java.util.LinkedHashMap; +import java.util.Map; + +/** Shared Aliyun DLF property binding and neutral catalog configuration. */ +public abstract class AbstractDlfMetaStoreProperties extends AbstractMetaStoreProperties + implements DlfMetaStoreProperties { + + // These aliases must stay aligned with both OSS binders so one credential set reaches metadata and storage. + @ConnectorProperty(names = {"dlf.access_key", "dlf.catalog.accessKeyId"}, required = false, sensitive = true, + description = "DLF access key id.") + private String accessKey = ""; + + @ConnectorProperty(names = {"dlf.secret_key", "dlf.catalog.secret_key", "dlf.catalog.accessKeySecret"}, + required = false, sensitive = true, + description = "DLF access key secret.") + private String secretKey = ""; + + @ConnectorProperty(names = {"dlf.session_token", "dlf.catalog.sessionToken", "dlf.catalog.securityToken"}, + required = false, sensitive = true, + description = "DLF session/security token.") + private String sessionToken = ""; + + @ConnectorProperty(names = {"dlf.region"}, required = false, + description = "DLF region used to derive the endpoint when it is not set.") + private String region = ""; + + @ConnectorProperty(names = {"dlf.endpoint", "dlf.catalog.endpoint"}, required = false, + description = "DLF endpoint.") + private String endpoint = ""; + + @ConnectorProperty(names = {"dlf.catalog.uid", "dlf.uid"}, required = false, + description = "DLF account uid.") + private String uid = ""; + + @ConnectorProperty(names = {"dlf.catalog.id", "dlf.catalog_id"}, required = false, + description = "DLF catalog id, defaulting to the uid.") + private String catalogId = ""; + + @ConnectorProperty(names = {"dlf.access.public", "dlf.catalog.accessPublic"}, required = false, + description = "Whether to use the public DLF endpoint instead of the VPC endpoint.") + private String accessPublic = "false"; + + @ConnectorProperty(names = {"dlf.catalog.proxyMode", "dlf.proxy.mode"}, required = false, + description = "DLF proxy mode.") + private String proxyMode = "DLF_ONLY"; + + private final Map<String, String> storageHadoopConfig; + + protected AbstractDlfMetaStoreProperties(Map<String, String> raw, Map<String, String> storageHadoopConfig) { + super(raw); + this.storageHadoopConfig = storageHadoopConfig; + } + + @Override + public String providerName() { + return "DLF"; + } + + @Override + public boolean needsStorage() { + return true; + } + + protected void validateConnection() { + if (StringUtils.isBlank(accessKey)) { + throw new IllegalArgumentException("dlf.access_key is required"); + } + if (StringUtils.isBlank(secretKey)) { + throw new IllegalArgumentException("dlf.secret_key is required"); + } + if (StringUtils.isBlank(endpoint) && StringUtils.isBlank(region)) { + throw new IllegalArgumentException("dlf.endpoint is required."); + } + } + + @Override + public Map<String, String> toDlfCatalogConf() { + String resolvedEndpoint = endpoint; + if (StringUtils.isBlank(resolvedEndpoint) && StringUtils.isNotBlank(region)) { + resolvedEndpoint = BooleanUtils.toBoolean(accessPublic) Review Comment: Fixed. DLF metastore and native OSS binding now use the same validated legacy-compatible boolean parser, including yes, on, and y spellings case-insensitively; invalid values are rejected instead of silently selecting different endpoints. Added tests for both metadata and storage binding. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
