Copilot commented on code in PR #61094:
URL: https://github.com/apache/doris/pull/61094#discussion_r2893436384
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonSysExternalTable.java:
##########
@@ -198,15 +198,16 @@ public TTableDescriptor toThrift() {
if (PaimonExternalCatalog.PAIMON_HMS.equals(catalogType)
|| PaimonExternalCatalog.PAIMON_FILESYSTEM.equals(catalogType)
|| PaimonExternalCatalog.PAIMON_DLF.equals(catalogType)
- || PaimonExternalCatalog.PAIMON_REST.equals(catalogType)) {
+ || PaimonExternalCatalog.PAIMON_REST.equals(catalogType)
+ || PaimonExternalCatalog.PAIMON_JDBC.equals(catalogType)) {
THiveTable tHiveTable = new THiveTable(dbName, name, new
HashMap<>());
TTableDescriptor tTableDescriptor = new TTableDescriptor(getId(),
TTableType.HIVE_TABLE, schema.size(), 0,
getName(), dbName);
tTableDescriptor.setHiveTable(tHiveTable);
return tTableDescriptor;
} else {
throw new IllegalArgumentException(
- "Currently only supports hms/dlf/rest/filesystem catalog,
do not support :" + catalogType);
+ "Currently only supports hms/dlf/rest/filesystem/jdbc
catalog, do not support :" + catalogType);
Review Comment:
Error message formatting: the string contains an extra space before the
colon (`"do not support :"`), which reads awkwardly and makes log/error
matching harder. Consider changing to `"do not support: " + catalogType` (also
adding a space after the colon for readability).
```suggestion
"Currently only supports hms/dlf/rest/filesystem/jdbc
catalog, do not support: " + catalogType);
```
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/PaimonJdbcMetaStoreProperties.java:
##########
@@ -0,0 +1,226 @@
+// 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.datasource.property.metastore;
+
+import org.apache.doris.catalog.JdbcResource;
+import
org.apache.doris.common.security.authentication.HadoopExecutionAuthenticator;
+import org.apache.doris.datasource.paimon.PaimonExternalCatalog;
+import org.apache.doris.datasource.property.ConnectorProperty;
+import org.apache.doris.datasource.property.storage.HdfsProperties;
+import org.apache.doris.datasource.property.storage.StorageProperties;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.paimon.catalog.Catalog;
+import org.apache.paimon.catalog.CatalogContext;
+import org.apache.paimon.catalog.CatalogFactory;
+import org.apache.paimon.jdbc.JdbcCatalogFactory;
+import org.apache.paimon.options.CatalogOptions;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class PaimonJdbcMetaStoreProperties extends AbstractPaimonProperties {
+ private static final Logger LOG =
LogManager.getLogger(PaimonJdbcMetaStoreProperties.class);
+ private static final Map<URL, ClassLoader> DRIVER_CLASS_LOADER_CACHE = new
ConcurrentHashMap<>();
+
+ @ConnectorProperty(
+ names = {"uri", "paimon.jdbc.uri"},
+ required = true,
+ description = "JDBC connection URI for the Paimon JDBC catalog."
+ )
+ private String uri = "";
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.user", "jdbc.user"},
+ required = false,
+ description = "Username for the Paimon JDBC catalog."
+ )
+ private String jdbcUser;
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.password", "jdbc.password"},
+ required = false,
+ sensitive = true,
+ description = "Password for the Paimon JDBC catalog."
+ )
+ private String jdbcPassword;
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.driver_url"},
+ required = false,
+ description = "JDBC driver JAR file path or URL. "
+ + "Can be a local file name (will look in
$DORIS_HOME/plugins/jdbc_drivers/) "
+ + "or a full URL (http://, https://, file://)."
+ )
+ private String driverUrl;
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.driver_class"},
+ required = false,
+ description = "JDBC driver class name. If specified with
paimon.jdbc.driver_url, "
+ + "the driver will be loaded dynamically."
+ )
+ private String driverClass;
+
+ protected PaimonJdbcMetaStoreProperties(Map<String, String> props) {
+ super(props);
+ }
+
+ @Override
+ public String getPaimonCatalogType() {
+ return PaimonExternalCatalog.PAIMON_JDBC;
+ }
+
+ @Override
+ protected void checkRequiredProperties() {
+ super.checkRequiredProperties();
+ if (StringUtils.isBlank(warehouse)) {
+ throw new IllegalArgumentException("Property warehouse is
required.");
+ }
+ }
+
+ @Override
+ public Catalog initializeCatalog(String catalogName,
List<StorageProperties> storagePropertiesList) {
+ buildCatalogOptions();
+ Configuration conf = new Configuration();
+ for (StorageProperties storageProperties : storagePropertiesList) {
+ if (storageProperties.getHadoopStorageConfig() != null) {
+ conf.addResource(storageProperties.getHadoopStorageConfig());
+ }
+ if
(storageProperties.getType().equals(StorageProperties.Type.HDFS)) {
+ this.executionAuthenticator = new
HadoopExecutionAuthenticator(((HdfsProperties) storageProperties)
+ .getHadoopAuthenticator());
+ }
+ }
+ appendUserHadoopConfig(conf);
+ if (StringUtils.isNotBlank(driverUrl)) {
+ registerJdbcDriver(driverUrl, driverClass);
+ LOG.info("Using dynamic JDBC driver for Paimon JDBC catalog from:
{}", driverUrl);
+ }
+ CatalogContext catalogContext = CatalogContext.create(catalogOptions,
conf);
+ try {
+ return this.executionAuthenticator.execute(() ->
CatalogFactory.createCatalog(catalogContext));
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to create Paimon catalog with
JDBC metastore: " + e.getMessage(), e);
+ }
+ }
+
+ @Override
+ protected void appendCustomCatalogOptions() {
+ catalogOptions.set(CatalogOptions.URI.key(), uri);
+ addIfNotBlank("jdbc.user", jdbcUser);
+ addIfNotBlank("jdbc.password", jdbcPassword);
+ }
+
+ @Override
+ protected String getMetastoreType() {
+ return JdbcCatalogFactory.IDENTIFIER;
+ }
+
+ private void addIfNotBlank(String key, String value) {
+ if (StringUtils.isNotBlank(value)) {
+ catalogOptions.set(key, value);
+ }
+ }
+
+ /**
+ * Register JDBC driver with DriverManager.
+ * This is necessary because DriverManager.getConnection() doesn't use
Thread.contextClassLoader.
+ */
+ private void registerJdbcDriver(String driverUrl, String driverClassName) {
+ try {
+ String fullDriverUrl = JdbcResource.getFullDriverUrl(driverUrl);
+ URL url = new URL(fullDriverUrl);
+
+ ClassLoader classLoader =
DRIVER_CLASS_LOADER_CACHE.computeIfAbsent(url, u -> {
+ ClassLoader parent = getClass().getClassLoader();
+ return URLClassLoader.newInstance(new URL[] {u}, parent);
+ });
+
+ if (StringUtils.isBlank(driverClassName)) {
+ throw new IllegalArgumentException(
+ "paimon.jdbc.driver_class is required when
paimon.jdbc.driver_url is specified");
+ }
+
+ Class<?> loadedDriverClass = Class.forName(driverClassName, true,
classLoader);
+ java.sql.Driver driver = (java.sql.Driver)
loadedDriverClass.getDeclaredConstructor().newInstance();
+ java.sql.DriverManager.registerDriver(new DriverShim(driver));
+ LOG.info("Successfully registered JDBC driver for Paimon catalog:
{} from {}",
+ driverClassName, fullDriverUrl);
Review Comment:
`registerJdbcDriver` calls `DriverManager.registerDriver(new
DriverShim(driver))` every time the catalog is initialized. Since
`DriverManager` keeps a global static list, repeated initialization (or
multiple catalogs using the same driver URL/class) will accumulate duplicate
driver instances and classloaders, leading to memory/permgen pressure and
slower driver resolution. Consider tracking already-registered (driverUrl,
driverClass) pairs (or checking `DriverManager.getDrivers()` for an existing
shim) and skipping registration when it’s already registered.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/PaimonJdbcMetaStoreProperties.java:
##########
@@ -0,0 +1,226 @@
+// 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.datasource.property.metastore;
+
+import org.apache.doris.catalog.JdbcResource;
+import
org.apache.doris.common.security.authentication.HadoopExecutionAuthenticator;
+import org.apache.doris.datasource.paimon.PaimonExternalCatalog;
+import org.apache.doris.datasource.property.ConnectorProperty;
+import org.apache.doris.datasource.property.storage.HdfsProperties;
+import org.apache.doris.datasource.property.storage.StorageProperties;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.paimon.catalog.Catalog;
+import org.apache.paimon.catalog.CatalogContext;
+import org.apache.paimon.catalog.CatalogFactory;
+import org.apache.paimon.jdbc.JdbcCatalogFactory;
+import org.apache.paimon.options.CatalogOptions;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class PaimonJdbcMetaStoreProperties extends AbstractPaimonProperties {
+ private static final Logger LOG =
LogManager.getLogger(PaimonJdbcMetaStoreProperties.class);
+ private static final Map<URL, ClassLoader> DRIVER_CLASS_LOADER_CACHE = new
ConcurrentHashMap<>();
+
+ @ConnectorProperty(
+ names = {"uri", "paimon.jdbc.uri"},
+ required = true,
+ description = "JDBC connection URI for the Paimon JDBC catalog."
+ )
+ private String uri = "";
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.user", "jdbc.user"},
+ required = false,
+ description = "Username for the Paimon JDBC catalog."
+ )
+ private String jdbcUser;
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.password", "jdbc.password"},
+ required = false,
+ sensitive = true,
+ description = "Password for the Paimon JDBC catalog."
+ )
+ private String jdbcPassword;
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.driver_url"},
+ required = false,
+ description = "JDBC driver JAR file path or URL. "
+ + "Can be a local file name (will look in
$DORIS_HOME/plugins/jdbc_drivers/) "
+ + "or a full URL (http://, https://, file://)."
+ )
+ private String driverUrl;
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.driver_class"},
+ required = false,
+ description = "JDBC driver class name. If specified with
paimon.jdbc.driver_url, "
+ + "the driver will be loaded dynamically."
+ )
+ private String driverClass;
+
+ protected PaimonJdbcMetaStoreProperties(Map<String, String> props) {
+ super(props);
+ }
+
+ @Override
+ public String getPaimonCatalogType() {
+ return PaimonExternalCatalog.PAIMON_JDBC;
+ }
+
+ @Override
+ protected void checkRequiredProperties() {
+ super.checkRequiredProperties();
+ if (StringUtils.isBlank(warehouse)) {
+ throw new IllegalArgumentException("Property warehouse is
required.");
+ }
+ }
+
+ @Override
+ public Catalog initializeCatalog(String catalogName,
List<StorageProperties> storagePropertiesList) {
+ buildCatalogOptions();
+ Configuration conf = new Configuration();
+ for (StorageProperties storageProperties : storagePropertiesList) {
+ if (storageProperties.getHadoopStorageConfig() != null) {
+ conf.addResource(storageProperties.getHadoopStorageConfig());
+ }
+ if
(storageProperties.getType().equals(StorageProperties.Type.HDFS)) {
+ this.executionAuthenticator = new
HadoopExecutionAuthenticator(((HdfsProperties) storageProperties)
+ .getHadoopAuthenticator());
+ }
+ }
+ appendUserHadoopConfig(conf);
+ if (StringUtils.isNotBlank(driverUrl)) {
+ registerJdbcDriver(driverUrl, driverClass);
+ LOG.info("Using dynamic JDBC driver for Paimon JDBC catalog from:
{}", driverUrl);
+ }
+ CatalogContext catalogContext = CatalogContext.create(catalogOptions,
conf);
+ try {
+ return this.executionAuthenticator.execute(() ->
CatalogFactory.createCatalog(catalogContext));
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to create Paimon catalog with
JDBC metastore: " + e.getMessage(), e);
+ }
+ }
+
+ @Override
+ protected void appendCustomCatalogOptions() {
+ catalogOptions.set(CatalogOptions.URI.key(), uri);
+ addIfNotBlank("jdbc.user", jdbcUser);
+ addIfNotBlank("jdbc.password", jdbcPassword);
+ }
+
+ @Override
+ protected String getMetastoreType() {
+ return JdbcCatalogFactory.IDENTIFIER;
+ }
+
+ private void addIfNotBlank(String key, String value) {
+ if (StringUtils.isNotBlank(value)) {
+ catalogOptions.set(key, value);
+ }
+ }
+
+ /**
+ * Register JDBC driver with DriverManager.
+ * This is necessary because DriverManager.getConnection() doesn't use
Thread.contextClassLoader.
+ */
+ private void registerJdbcDriver(String driverUrl, String driverClassName) {
+ try {
+ String fullDriverUrl = JdbcResource.getFullDriverUrl(driverUrl);
+ URL url = new URL(fullDriverUrl);
+
+ ClassLoader classLoader =
DRIVER_CLASS_LOADER_CACHE.computeIfAbsent(url, u -> {
+ ClassLoader parent = getClass().getClassLoader();
+ return URLClassLoader.newInstance(new URL[] {u}, parent);
+ });
+
+ if (StringUtils.isBlank(driverClassName)) {
+ throw new IllegalArgumentException(
+ "paimon.jdbc.driver_class is required when
paimon.jdbc.driver_url is specified");
+ }
+
+ Class<?> loadedDriverClass = Class.forName(driverClassName, true,
classLoader);
+ java.sql.Driver driver = (java.sql.Driver)
loadedDriverClass.getDeclaredConstructor().newInstance();
+ java.sql.DriverManager.registerDriver(new DriverShim(driver));
+ LOG.info("Successfully registered JDBC driver for Paimon catalog:
{} from {}",
+ driverClassName, fullDriverUrl);
+ } catch (MalformedURLException e) {
+ throw new IllegalArgumentException("Invalid driver URL: " +
driverUrl, e);
+ } catch (ClassNotFoundException e) {
+ throw new IllegalArgumentException("Failed to load JDBC driver
class: " + driverClassName, e);
+ } catch (IllegalArgumentException e) {
+ throw e;
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to register JDBC driver: " +
driverClassName, e);
+ }
+ }
+
+ private static class DriverShim implements java.sql.Driver {
+ private final java.sql.Driver delegate;
+
+ DriverShim(java.sql.Driver delegate) {
+ this.delegate = delegate;
Review Comment:
The nested `DriverShim` is duplicated in `IcebergJdbcMetaStoreProperties`
with the same behavior. Consider extracting a shared utility/shim to avoid
divergence when future fixes are needed (e.g., preventing duplicate
registrations, cache invalidation).
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalTable.java:
##########
@@ -181,15 +181,16 @@ public TTableDescriptor toThrift() {
if (PaimonExternalCatalog.PAIMON_HMS.equals(getPaimonCatalogType())
||
PaimonExternalCatalog.PAIMON_FILESYSTEM.equals(getPaimonCatalogType())
||
PaimonExternalCatalog.PAIMON_DLF.equals(getPaimonCatalogType())
- ||
PaimonExternalCatalog.PAIMON_REST.equals(getPaimonCatalogType())) {
+ ||
PaimonExternalCatalog.PAIMON_REST.equals(getPaimonCatalogType())
+ ||
PaimonExternalCatalog.PAIMON_JDBC.equals(getPaimonCatalogType())) {
THiveTable tHiveTable = new THiveTable(dbName, name, new
HashMap<>());
TTableDescriptor tTableDescriptor = new TTableDescriptor(getId(),
TTableType.HIVE_TABLE, schema.size(), 0,
getName(), dbName);
tTableDescriptor.setHiveTable(tHiveTable);
return tTableDescriptor;
} else {
throw new IllegalArgumentException(
- "Currently only supports hms/dlf/rest/filesystem catalog,
do not support :"
+ "Currently only supports hms/dlf/rest/filesystem/jdbc
catalog, do not support :"
Review Comment:
Error message formatting: this string includes `"do not support :"` (space
before colon). Consider changing to `"do not support: " +
getPaimonCatalogType()` for consistent, cleaner messages.
```suggestion
"Currently only supports hms/dlf/rest/filesystem/jdbc
catalog, do not support: "
```
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/PaimonJdbcMetaStoreProperties.java:
##########
@@ -0,0 +1,226 @@
+// 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.datasource.property.metastore;
+
+import org.apache.doris.catalog.JdbcResource;
+import
org.apache.doris.common.security.authentication.HadoopExecutionAuthenticator;
+import org.apache.doris.datasource.paimon.PaimonExternalCatalog;
+import org.apache.doris.datasource.property.ConnectorProperty;
+import org.apache.doris.datasource.property.storage.HdfsProperties;
+import org.apache.doris.datasource.property.storage.StorageProperties;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.paimon.catalog.Catalog;
+import org.apache.paimon.catalog.CatalogContext;
+import org.apache.paimon.catalog.CatalogFactory;
+import org.apache.paimon.jdbc.JdbcCatalogFactory;
+import org.apache.paimon.options.CatalogOptions;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class PaimonJdbcMetaStoreProperties extends AbstractPaimonProperties {
+ private static final Logger LOG =
LogManager.getLogger(PaimonJdbcMetaStoreProperties.class);
+ private static final Map<URL, ClassLoader> DRIVER_CLASS_LOADER_CACHE = new
ConcurrentHashMap<>();
+
+ @ConnectorProperty(
+ names = {"uri", "paimon.jdbc.uri"},
+ required = true,
+ description = "JDBC connection URI for the Paimon JDBC catalog."
+ )
+ private String uri = "";
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.user", "jdbc.user"},
+ required = false,
+ description = "Username for the Paimon JDBC catalog."
+ )
+ private String jdbcUser;
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.password", "jdbc.password"},
+ required = false,
+ sensitive = true,
+ description = "Password for the Paimon JDBC catalog."
+ )
+ private String jdbcPassword;
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.driver_url"},
+ required = false,
+ description = "JDBC driver JAR file path or URL. "
+ + "Can be a local file name (will look in
$DORIS_HOME/plugins/jdbc_drivers/) "
+ + "or a full URL (http://, https://, file://)."
+ )
+ private String driverUrl;
+
+ @ConnectorProperty(
+ names = {"paimon.jdbc.driver_class"},
+ required = false,
+ description = "JDBC driver class name. If specified with
paimon.jdbc.driver_url, "
+ + "the driver will be loaded dynamically."
+ )
+ private String driverClass;
+
+ protected PaimonJdbcMetaStoreProperties(Map<String, String> props) {
+ super(props);
+ }
+
+ @Override
+ public String getPaimonCatalogType() {
+ return PaimonExternalCatalog.PAIMON_JDBC;
+ }
+
+ @Override
+ protected void checkRequiredProperties() {
+ super.checkRequiredProperties();
+ if (StringUtils.isBlank(warehouse)) {
+ throw new IllegalArgumentException("Property warehouse is
required.");
+ }
+ }
+
+ @Override
+ public Catalog initializeCatalog(String catalogName,
List<StorageProperties> storagePropertiesList) {
+ buildCatalogOptions();
+ Configuration conf = new Configuration();
+ for (StorageProperties storageProperties : storagePropertiesList) {
+ if (storageProperties.getHadoopStorageConfig() != null) {
+ conf.addResource(storageProperties.getHadoopStorageConfig());
+ }
+ if
(storageProperties.getType().equals(StorageProperties.Type.HDFS)) {
+ this.executionAuthenticator = new
HadoopExecutionAuthenticator(((HdfsProperties) storageProperties)
+ .getHadoopAuthenticator());
+ }
+ }
+ appendUserHadoopConfig(conf);
+ if (StringUtils.isNotBlank(driverUrl)) {
+ registerJdbcDriver(driverUrl, driverClass);
+ LOG.info("Using dynamic JDBC driver for Paimon JDBC catalog from:
{}", driverUrl);
+ }
+ CatalogContext catalogContext = CatalogContext.create(catalogOptions,
conf);
+ try {
+ return this.executionAuthenticator.execute(() ->
CatalogFactory.createCatalog(catalogContext));
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to create Paimon catalog with
JDBC metastore: " + e.getMessage(), e);
+ }
+ }
+
+ @Override
+ protected void appendCustomCatalogOptions() {
+ catalogOptions.set(CatalogOptions.URI.key(), uri);
+ addIfNotBlank("jdbc.user", jdbcUser);
+ addIfNotBlank("jdbc.password", jdbcPassword);
+ }
+
+ @Override
+ protected String getMetastoreType() {
+ return JdbcCatalogFactory.IDENTIFIER;
+ }
+
+ private void addIfNotBlank(String key, String value) {
+ if (StringUtils.isNotBlank(value)) {
+ catalogOptions.set(key, value);
+ }
+ }
+
+ /**
+ * Register JDBC driver with DriverManager.
+ * This is necessary because DriverManager.getConnection() doesn't use
Thread.contextClassLoader.
+ */
+ private void registerJdbcDriver(String driverUrl, String driverClassName) {
+ try {
+ String fullDriverUrl = JdbcResource.getFullDriverUrl(driverUrl);
+ URL url = new URL(fullDriverUrl);
+
+ ClassLoader classLoader =
DRIVER_CLASS_LOADER_CACHE.computeIfAbsent(url, u -> {
+ ClassLoader parent = getClass().getClassLoader();
+ return URLClassLoader.newInstance(new URL[] {u}, parent);
+ });
Review Comment:
`DRIVER_CLASS_LOADER_CACHE.computeIfAbsent(...)` permanently caches a
classloader for a URL. If driver loading fails (bad jar, transient download
issue, wrong class), the cached entry can keep subsequent retries stuck using
the same (potentially poisoned) loader. Consider removing the cache entry on
failures (e.g., in the `ClassNotFoundException`/generic catch path) so users
can fix the driver and retry without restarting FE.
--
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]
