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]

Reply via email to