This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.0 by this push:
new 0e53575b7cb branch-4.0: [fix](jdbc) check old driver path before
downloading from cloud #59928 (#60038)
0e53575b7cb is described below
commit 0e53575b7cba68111468a3c9e3b12c3ee8949aa6
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue Jan 20 10:13:28 2026 +0800
branch-4.0: [fix](jdbc) check old driver path before downloading from cloud
#59928 (#60038)
Cherry-picked from #59928
Co-authored-by: Mingyu Chen (Rayner) <[email protected]>
---
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 9924df7b360..0a2aae591e2 100644
--- a/be/src/vec/exec/vjdbc_connector.cpp
+++ b/be/src/vec/exec/vjdbc_connector.cpp
@@ -681,10 +681,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;
@@ -698,10 +703,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]