This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 85f39d5813e [fix](jdbc) check old driver path before downloading from
cloud (#59928)
85f39d5813e is described below
commit 85f39d5813e78a21454b30640727c21e4742433d
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Mon Jan 19 22:34:14 2026 +0800
[fix](jdbc) check old driver path before downloading from cloud (#59928)
### What problem does this PR solve?
Related: #54304
Problem Summary:
Before this change, when a JDBC driver was not found in the new default
directory, the code would fallback to the old directory unconditionally,
even if the driver didn't exist there. In cloud mode, it would try to
download from cloud but then fallback to old directory regardless of
download result.
This commit improves the driver lookup logic:
1. Check new default directory first (DORIS_HOME/plugins/jdbc_drivers)
2. Check old default directory (DORIS_HOME/jdbc_drivers)
3. In cloud mode, only download from cloud if not found in either
directory
4. Return error immediately if driver not found (instead of fallback)
This avoids unnecessary cloud downloads when driver exists in old path,
and provides clearer error messages when driver is truly missing.
Changes:
- BE: vjdbc_connector.cpp - check old path before cloud download
- FE: JdbcResource.java - check old path and improve error messages
---
be/src/vec/exec/vjdbc_connector.cpp | 10 ++-
be/test/vec/exec/vjdbc_connector_test.cpp | 99 +++++++++++++++++++---
.../org/apache/doris/catalog/JdbcResource.java | 14 ++-
.../org/apache/doris/catalog/JdbcResourceTest.java | 22 +----
4 files changed, 109 insertions(+), 36 deletions(-)
diff --git a/be/src/vec/exec/vjdbc_connector.cpp
b/be/src/vec/exec/vjdbc_connector.cpp
index 38a83a01812..2878a7969df 100644
--- a/be/src/vec/exec/vjdbc_connector.cpp
+++ b/be/src/vec/exec/vjdbc_connector.cpp
@@ -654,10 +654,15 @@ Status
JdbcConnector::_check_and_return_default_driver_url(const std::string& ur
// from `DORIS_HOME/jdbc_drivers` to `DORIS_HOME/plugins/jdbc_drivers`,
// so we need to check the old default dir for compatibility.
std::string target_path = default_url + "/" + url;
+ std::string old_target_path = default_old_url + "/" + url;
if (std::filesystem::exists(target_path)) {
// File exists in new default directory
*result_url = "file://" + target_path;
return Status::OK();
+ } else if (std::filesystem::exists(old_target_path)) {
+ // File exists in old default directory
+ *result_url = "file://" + old_target_path;
+ return Status::OK();
} else if (config::is_cloud_mode()) {
// Cloud mode: try to download from cloud to new default directory
std::string downloaded_path;
@@ -671,10 +676,9 @@ Status
JdbcConnector::_check_and_return_default_driver_url(const std::string& ur
// Download failed, log warning but continue to fallback
LOG(WARNING) << "Failed to download JDBC driver from cloud: " <<
status.to_string()
<< ", fallback to old directory";
+ } else {
+ return Status::InternalError("JDBC driver file does not exist: " +
url);
}
-
- // Fallback to old default directory for compatibility
- *result_url = "file://" + default_old_url + "/" + url;
} else {
// User specified custom directory - use directly
*result_url = "file://" + config::jdbc_drivers_dir + "/" + url;
diff --git a/be/test/vec/exec/vjdbc_connector_test.cpp
b/be/test/vec/exec/vjdbc_connector_test.cpp
index 8d8ef085699..38b9f0004f3 100644
--- a/be/test/vec/exec/vjdbc_connector_test.cpp
+++ b/be/test/vec/exec/vjdbc_connector_test.cpp
@@ -110,13 +110,28 @@ TEST_F(JdbcConnectorTest,
TestCheckAndReturnDefaultDriverUrlWithDefaultConfig) {
// Set config to default value to trigger the default directory logic
config::jdbc_drivers_dir = "/tmp/test_doris/plugins/jdbc_drivers";
+ // Create the target directory and file for testing
+ std::string dir = "/tmp/test_doris/plugins/jdbc_drivers";
+ std::string file_path = dir + "/mysql-connector.jar";
+
+ // Create directory
+ std::filesystem::create_directories(dir);
+
+ // Create test file
+ std::ofstream file(file_path);
+ file << "test content";
+ file.close();
+
std::string result_url;
Status status =
connector._check_and_return_default_driver_url("mysql-connector.jar",
&result_url);
EXPECT_TRUE(status.ok());
- EXPECT_TRUE(result_url.find("file://") == 0);
- EXPECT_TRUE(result_url.find("mysql-connector.jar") != std::string::npos);
+ EXPECT_EQ(result_url, "file://" + file_path);
+
+ // Cleanup
+ std::filesystem::remove(file_path);
+ std::filesystem::remove_all(dir);
}
TEST_F(JdbcConnectorTest, TestCheckAndReturnDefaultDriverUrlWithCustomConfig) {
@@ -170,13 +185,28 @@ TEST_F(JdbcConnectorTest, TestCloudModeSimulation) {
// Set config to default value
config::jdbc_drivers_dir = "/tmp/test_doris/plugins/jdbc_drivers";
+ // Create the old directory and file for testing (fallback path)
+ std::string old_dir = "/tmp/test_doris/jdbc_drivers";
+ std::string file_path = old_dir + "/cloud-driver.jar";
+
+ // Create directory
+ std::filesystem::create_directories(old_dir);
+
+ // Create test file
+ std::ofstream file(file_path);
+ file << "test content";
+ file.close();
+
std::string result_url;
Status status =
connector._check_and_return_default_driver_url("cloud-driver.jar", &result_url);
// Should process successfully and return fallback path
EXPECT_TRUE(status.ok());
- EXPECT_TRUE(result_url.find("file://") == 0);
- EXPECT_TRUE(result_url.find("jdbc_drivers/cloud-driver.jar") !=
std::string::npos);
+ EXPECT_EQ(result_url, "file://" + file_path);
+
+ // Cleanup
+ std::filesystem::remove(file_path);
+ std::filesystem::remove_all(old_dir);
}
TEST_F(JdbcConnectorTest, TestFallbackToOldDirectory) {
@@ -185,29 +215,61 @@ TEST_F(JdbcConnectorTest, TestFallbackToOldDirectory) {
// Set config to default value but file doesn't exist in new directory
config::jdbc_drivers_dir = "/tmp/test_doris/plugins/jdbc_drivers";
+ // Create the old directory and file for testing (fallback path)
+ std::string old_dir = "/tmp/test_doris/jdbc_drivers";
+ std::string file_path = old_dir + "/fallback-driver.jar";
+
+ // Create directory
+ std::filesystem::create_directories(old_dir);
+
+ // Create test file
+ std::ofstream file(file_path);
+ file << "test content";
+ file.close();
+
std::string result_url;
Status status =
connector._check_and_return_default_driver_url("fallback-driver.jar",
&result_url);
EXPECT_TRUE(status.ok());
// Should fallback to old directory when file not found and not in cloud
mode
- EXPECT_EQ(result_url,
"file:///tmp/test_doris/jdbc_drivers/fallback-driver.jar");
+ EXPECT_EQ(result_url, "file://" + file_path);
+
+ // Cleanup
+ std::filesystem::remove(file_path);
+ std::filesystem::remove_all(old_dir);
}
TEST_F(JdbcConnectorTest, TestPathConstruction) {
auto connector = createConnector();
- // Test different DORIS_HOME values
- setenv("DORIS_HOME", "/test/doris", 1);
+ // Test different DORIS_HOME values (use /tmp to avoid permission issues)
+ setenv("DORIS_HOME", "/tmp/test_doris2", 1);
// Set to default config
- config::jdbc_drivers_dir = "/test/doris/plugins/jdbc_drivers";
+ config::jdbc_drivers_dir = "/tmp/test_doris2/plugins/jdbc_drivers";
+
+ // Create the old directory and file for testing (fallback path)
+ std::string old_dir = "/tmp/test_doris2/jdbc_drivers";
+ std::string file_path = old_dir + "/test.jar";
+
+ // Create directory
+ std::filesystem::create_directories(old_dir);
+
+ // Create test file
+ std::ofstream file(file_path);
+ file << "test content";
+ file.close();
std::string result_url;
Status status = connector._check_and_return_default_driver_url("test.jar",
&result_url);
EXPECT_TRUE(status.ok());
- EXPECT_EQ(result_url, "file:///test/doris/jdbc_drivers/test.jar"); //
Fallback path
+ EXPECT_EQ(result_url, "file://" + file_path); // Fallback path
+
+ // Cleanup
+ std::filesystem::remove(file_path);
+ std::filesystem::remove_all(old_dir);
}
TEST_F(JdbcConnectorTest, TestEdgeCases) {
@@ -232,6 +294,18 @@ TEST_F(JdbcConnectorTest, TestMultipleCallsConsistency) {
config::jdbc_drivers_dir = "/tmp/test_doris/plugins/jdbc_drivers";
+ // Create the target directory and file for testing
+ std::string dir = "/tmp/test_doris/plugins/jdbc_drivers";
+ std::string file_path = dir + "/same-driver.jar";
+
+ // Create directory
+ std::filesystem::create_directories(dir);
+
+ // Create test file
+ std::ofstream file(file_path);
+ file << "test content";
+ file.close();
+
std::string result_url1, result_url2;
Status status1 =
connector._check_and_return_default_driver_url("same-driver.jar",
&result_url1);
@@ -241,6 +315,11 @@ TEST_F(JdbcConnectorTest, TestMultipleCallsConsistency) {
EXPECT_TRUE(status1.ok());
EXPECT_TRUE(status2.ok());
EXPECT_EQ(result_url1, result_url2); // Should be consistent
+ EXPECT_EQ(result_url1, "file://" + file_path);
+
+ // Cleanup
+ std::filesystem::remove(file_path);
+ std::filesystem::remove_all(dir);
}
TEST_F(JdbcConnectorTest, TestUrlDetectionLogic) {
@@ -269,4 +348,4 @@ TEST_F(JdbcConnectorTest, TestUrlDetectionLogic) {
}
}
-} // namespace doris::vectorized
\ No newline at end of file
+} // namespace doris::vectorized
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
index c9d21e7174d..b726d4ad60f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
@@ -336,9 +336,14 @@ public class JdbcResource extends Resource {
// so we need to check the old default dir for compatibility.
String targetPath = defaultDriverUrl + "/" + driverUrl;
File targetFile = new File(targetPath);
+ String oldTargetPath = defaultOldDriverUrl + "/" + driverUrl;
+ File oldTargetFile = new File(oldTargetPath);
if (targetFile.exists()) {
// File exists in new default directory
return "file://" + targetPath;
+ } else if (oldTargetFile.exists()) {
+ // File exists in old default directory
+ return "file://" + oldTargetPath;
} else if (Config.isCloudMode()) {
// Cloud mode: download from cloud to default directory
try {
@@ -346,12 +351,15 @@ public class JdbcResource extends Resource {
PluginType.JDBC_DRIVERS, driverUrl, targetPath);
return "file://" + downloadedPath;
} catch (Exception e) {
+ LOG.warn("failed to download jdbc driver url: " +
driverUrl, e);
throw new RuntimeException("Cannot download JDBC driver
from cloud: " + driverUrl
- + ". Please retry later or check your driver has
been uploaded to cloud.");
+ + ". Please retry later or check your driver has
been uploaded to cloud. Error: "
+ + Util.getRootCauseMessage(e));
}
+ } else {
+ // File does not exist in both new and old default directory
+ throw new RuntimeException("JDBC driver file does not exist: "
+ driverUrl);
}
- // Fallback to old default directory for compatibility
- return "file://" + defaultOldDriverUrl + "/" + driverUrl;
} else {
// Return user specified driver url directly.
return "file://" + Config.jdbc_drivers_dir + "/" + driverUrl;
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/catalog/JdbcResourceTest.java
b/fe/fe-core/src/test/java/org/apache/doris/catalog/JdbcResourceTest.java
index d564f79c49d..2e2d9af9626 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/catalog/JdbcResourceTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/JdbcResourceTest.java
@@ -17,9 +17,7 @@
package org.apache.doris.catalog;
-import org.apache.doris.common.Config;
import org.apache.doris.common.DdlException;
-import org.apache.doris.common.EnvUtils;
import org.apache.doris.common.FeConstants;
import org.apache.doris.common.UserException;
import org.apache.doris.mysql.privilege.AccessControllerManager;
@@ -200,21 +198,6 @@ public class JdbcResourceTest {
Assert.assertTrue(resultUrl.contains(";"));
}
- @Test
- public void testJdbcDriverPath() {
- String driverPath = "postgresql-42.5.0.jar";
- Config.jdbc_driver_secure_path = "";
- Config.jdbc_drivers_dir = EnvUtils.getDorisHome() +
"/plugins/jdbc_drivers";
- String fullPath = JdbcResource.getFullDriverUrl(driverPath);
- Assert.assertEquals("file://" + EnvUtils.getDorisHome() +
"/jdbc_drivers/" + driverPath, fullPath);
- Config.jdbc_driver_secure_path = "file:///jdbc/;http://jdbc";;
- String driverPath2 = "file:///postgresql-42.5.0.jar";
- Exception exception =
Assert.assertThrows(IllegalArgumentException.class, () -> {
- JdbcResource.getFullDriverUrl(driverPath2);
- });
- Assert.assertEquals("Driver URL does not match any allowed paths:
file:///postgresql-42.5.0.jar", exception.getMessage());
- }
-
@Test
public void testValidDriverUrls() {
String fileUrl = "file://path/to/driver.jar";
@@ -236,9 +219,8 @@ public class JdbcResourceTest {
});
String jarFile = "driver.jar";
- Assertions.assertDoesNotThrow(() -> {
- String result = JdbcResource.getFullDriverUrl(jarFile);
- Assert.assertTrue(result.startsWith("file://"));
+ Assertions.assertThrows(RuntimeException.class, () -> {
+ JdbcResource.getFullDriverUrl(jarFile);
});
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
