Fix MSSQL JDBC "database=" parameter not extracting db.name, issue 19024#19029
Open
hwxy233 wants to merge 13 commits into
Open
Fix MSSQL JDBC "database=" parameter not extracting db.name, issue 19024#19029hwxy233 wants to merge 13 commits into
hwxy233 wants to merge 13 commits into
Conversation
The Microsoft SQL Server JDBC driver supports both 'databaseName' and 'database' as connection property names for specifying the database name. However, the URL parser only checked for 'databasename' (after lowercasing), causing jdbc:sqlserver://...;database=xxx URLs to not extract the db.name attribute. This commit adds 'database' as a fallback key in: - ParseContext.applyCommonParams() - used by MssqlUrlParser - JtdsUrlParser.parse() - used by jTDS driver Additional test cases cover the 'database=' parameter for both sqlserver and jtds URL formats.
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds support for the database= connection string property as an alias for databaseName in SQL Server and jTDS JDBC URL parsing.
Changes:
- Adds fallback logic in
ParseContext.applyCommonParamsto check thedatabaseparameter whendatabasenameis null or empty. - Updates
JtdsUrlParserto also resolve thedatabaseparameter as a fallback when no path-based database name is found. - Adds test cases covering the
database=alias in both standard SQL Server and jTDS connection URLs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
ParseContext.java |
Falls back to database param if databasename is null or empty in applyCommonParams. |
JtdsUrlParser.java |
Adds database as a fallback parameter when resolving database name from jTDS URL params. |
JdbcConnectionUrlParserTest.java |
Adds test cases for the database= alias in SQL Server and jTDS URLs. |
…th ParseContext Previously JtdsUrlParser used containsKey() which would accept an empty 'databasename=' value and not fall through to the 'database' fallback. Now uses the same null/empty check pattern as ParseContext.applyCommonParams: if databasename is absent or empty, fall through to the database parameter.
…method Replace duplicated resolution logic in both ParseContext.applyCommonParams() and JtdsUrlParser.parse() with a single ParseContext.applyDatabaseNameParam(Map) method that resolves the database name from URL parameters, trying 'databasename' first and falling back to 'database'.
…/hwxy233/opentelemetry-java-instrumentation into bugfix-mssql-jdbc-database-param
… parsing in MssqlUrlParser Addresses two review comments: 1. Deduplicate getDatabaseNameParam: The method was copied verbatim in both MssqlUrlParser and JtdsUrlParser. Moved to UrlParsingUtils as a shared static helper, consistent with other utilities like extractSubtype and splitQuery already in that class. 2. Eliminate redundant URL parsing: MssqlUrlParser was parsing semicolon parameters twice — once in applyCommonParams and again in applyDatabaseAliasParam. Now extract the params map once in parse(), then pass it to both applyCommonParams (new map-based overload) and applyDatabaseAliasParam. Additionally, applyDatabaseAliasParam no longer re-extracts params from the URL string.
…UrlParsingUtils
…e from MssqlUrlParser Refactor suggested by review comment: instead of duplicating the applyCommonParams logic as a private method in MssqlUrlParser, add a public overload ParseContext.applyCommonParams(Map<String, String>) that accepts a pre-parsed parameter map. MssqlUrlParser now calls ctx.applyCommonParams(urlParams) directly, eliminating the near-copy. The original applyCommonParams(String, String, String) now delegates to the new map-based overload to ensure both versions stay in sync.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For this issue The Java agent did not detect the database name, I want to create a pr.
The Microsoft SQL Server JDBC driver supports both 'databaseName' and 'database' as connection property names for specifying the database name. However, the URL parser only checked for 'databasename' (after lowercasing), causing jdbc:sqlserver://...;database=xxx URLs to not extract the db.name attribute.
This commit adds 'database' as a fallback key in:
Additional test cases cover the 'database=' parameter for both sqlserver and jtds URL formats.