You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "terrymanu (via GitHub)" <gi...@apache.org> on 2026/07/05 14:50:35 UTC

[PR] Refactor database type detection to use metadata fallback SPI [shardingsphere]

terrymanu opened a new pull request, #39013:
URL: https://github.com/apache/shardingsphere/pull/39013

   (no comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Refactor database type detection to use metadata fallback SPI [shardingsphere]

Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #39013:
URL: https://github.com/apache/shardingsphere/pull/39013


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Refactor database type detection to use metadata fallback SPI [shardingsphere]

Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39013:
URL: https://github.com/apache/shardingsphere/pull/39013#issuecomment-4886549622

   ### Summary
   
   **Review Result: Mergeable**
   
   Reason: This is a code-scope review only; CI not reviewed by request. The latest PR head centralizes metadata-based database type fallback in `DatabaseTypeFactory`, removes the Hive-specific branch from shared core code, updates callers to use the metadata overload, and includes focused tests for the new fallback and exception boundaries. I did not find code-level blockers in the reviewed scope.
   
   ### Evidence
   
   - `database/connector/core/src/main/java/org/apache/shardingsphere/database/connector/core/type/DatabaseTypeFactory.java#L65-L79` now keeps the complexity inside `get(DatabaseMetaData)`: it first resolves by `DatabaseMetaData#getURL()`, then falls back through `DialectJdbcUrlFetcher` only when the URL operation is unsupported.
   - `database/connector/dialect/hive/src/main/java/org/apache/shardingsphere/database/connector/hive/jdbcurl/HiveJdbcUrlFetcher.java#L31-L39` and `database/connector/dialect/hive/src/main/resources/META-INF/services/org.apache.shardingsphere.database.connector.core.jdbcurl.DialectJdbcUrlFetcher#L18` provide the Hive-specific URL extraction through SPI, so shared core code no longer needs a Hive type-name check.
   - `infra/common/src/main/java/org/apache/shardingsphere/infra/database/DatabaseTypeEngine.java#L99-L103`, `kernel/transaction/core/src/main/java/org/apache/shardingsphere/transaction/savepoint/ConnectionSavepointManager.java#L100-L102`, and `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/metadata/jdbc/MCPJdbcDatabaseProfileLoader.java#L60-L75` now delegate to `DatabaseTypeFactory.get(DatabaseMetaData)`, preserving the intended boundary that callers do not need to know the dialect fallback details.
   - The changed tests cover direct URL resolution, unsupported metadata URL fallback through `DialectJdbcUrlFetcher`, no matching fetcher, unsupported fetched URL, caller exception wrapping, and MCP invalid-configuration conversion.
   
   ### Review Details
   
   - Review Focus: Code Correctness Review. CI not reviewed by request.
   - Reviewed Scope: PR #39013, latest head `b000406db8d885176399e7958724f9e4d5f13485`, base `master` at `b1508805707a53220e23290910d3699f747a8c2d`, merge-base `b1508805707a53220e23290910d3699f747a8c2d`. Reviewed all 9 files from GitHub `/pulls/39013/files`; the local triple-dot file list matched the GitHub file list.
   - Not Reviewed Scope: GitHub Actions/check-runs/logs, full E2E suites, native-image/distribution packaging beyond the changed files.
   - Verification:
     - Public GitHub PR metadata, file list, issue comments, PR reviews, and review comments were accessible through GitHub REST. Public comments/reviews were empty at review time.
     - `./mvnw -pl database/connector/core,infra/common,kernel/transaction/core,mcp/support -Dtest=DatabaseTypeFactoryTest,DatabaseTypeEngineTest,ConnectionSavepointManagerTest,MCPJdbcDatabaseProfileLoaderTest test -Pcheck -T1C` passed with exit code 0.
     - `./mvnw -pl mcp/core test -Pcheck -T1C` passed with exit code 0.
     - `./mvnw checkstyle:check -Pcheck -T1C` passed with exit code 0.
     - `./mvnw spotless:check -Pcheck -T1C` passed with exit code 0.
   - Release Note / User Docs: Not required. The PR is an internal refactor with no public configuration key, SQL syntax, user-facing protocol, migration, or compatibility contract change found in the reviewed scope.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org