-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix](jdbc) check old driver path before downloading from cloud #59928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
JNSimba
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
rum buildall |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
|
run buildall |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
|
PR approved by at least one committer and no changes requested. |
TPC-H: Total hot run time: 31467 ms |
TPC-DS: Total hot run time: 174337 ms |
ClickBench: Total hot run time: 26.83 s |
FE Regression Coverage ReportIncrement line coverage |
1 similar comment
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
TPC-H: Total hot run time: 32291 ms |
TPC-DS: Total hot run time: 174368 ms |
ClickBench: Total hot run time: 26.65 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run buildall |
TPC-H: Total hot run time: 32384 ms |
TPC-DS: Total hot run time: 177484 ms |
ClickBench: Total hot run time: 27.22 s |
FE UT Coverage ReportIncrement line coverage |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
|
PR approved by at least one committer and no changes requested. |
JNSimba
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
run check_coverage |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### 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
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:
This avoids unnecessary cloud downloads when driver exists in old path,
and provides clearer error messages when driver is truly missing.
Changes:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)