Skip to content

Align DistSQL handler package layout in kernel/distsql#6

Closed
Copilot wants to merge 6 commits intomasterfrom
copilot/migrate-distsql-modules
Closed

Align DistSQL handler package layout in kernel/distsql#6
Copilot wants to merge 6 commits intomasterfrom
copilot/migrate-distsql-modules

Conversation

Copy link

Copilot AI commented Jan 27, 2026

Relocates the migrated DistSQL executors into the finalized package layout for the new kernel/distsql module, matching the original proxy ral and rul namespaces.

Changes Made

  • Package layout: Move RAL queryable/updatable handlers under org.apache.shardingsphere.distsql.handler.ral.* and RUL handlers under org.apache.shardingsphere.distsql.handler.rul.*.
  • Source/test updates: Adjust package declarations, imports, and test locations to match the new namespaces.
  • SPI + reachability: Update SPI registration entries and native-image reachability metadata to the new package names.
Original prompt

The link apache#37778 (review) mentions that most DistSQL executors are located under proxy/backend/handler/distsql. Please migrate as many distsql-related modules as possible to the Maven submodules of https://github.com/apache/shardingsphere/tree/master/kernel to serve the usage of apache#37778 . This mainly involves org.apache.shardingsphere.distsql.**, org.apache.shardingsphere.proxy.backend.distsql.**, or other Java packages.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Migrate DistSQL executors to kernel Maven submodules Plan: migrate proxy DistSQL executors into kernel modules Jan 27, 2026
Copilot AI requested a review from linghengqian January 27, 2026 12:29
linghengqian

This comment was marked as outdated.

linghengqian

This comment was marked as outdated.

linghengqian

This comment was marked as outdated.

linghengqian

This comment was marked as outdated.

Co-authored-by: linghengqian <20187731+linghengqian@users.noreply.github.com>
Copilot AI changed the title Plan: migrate proxy DistSQL executors into kernel modules Migrate eligible proxy DistSQL executors into new kernel/distsql module Jan 27, 2026
Copilot AI requested a review from linghengqian January 27, 2026 16:17
linghengqian

This comment was marked as outdated.

Co-authored-by: linghengqian <20187731+linghengqian@users.noreply.github.com>
Co-authored-by: linghengqian <20187731+linghengqian@users.noreply.github.com>
Copy link
Author

Copilot AI commented Jan 28, 2026

I encountered errors locally when running ./mvnw clean install -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e and ./mvnw checkstyle:check -Pcheck -T1C using OpenJDK 24.0.2. You need to fix your ...

Fixed the migrated unit tests, updated reachability metadata, and ran Spotless, scoped tests, and Checkstyle. Latest commit: 4e113c4.

linghengqian

This comment was marked as outdated.

Co-authored-by: linghengqian <20187731+linghengqian@users.noreply.github.com>
Copy link
Author

Copilot AI commented Jan 28, 2026

I think you need to migrate or add SetDistVariableExecutorTest, RefreshDatabaseMetaDataExecutorTest, and DistSQLUpdateProxyBackendHandlerTest at the same time, regardless of which Maven module t...

Restored SetDistVariableExecutorTest, RefreshDatabaseMetaDataExecutorTest, and DistSQLUpdateProxyBackendHandlerTest in commit b48d4f8.

linghengqian

This comment was marked as outdated.

Copy link
Owner

@linghengqian linghengqian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirm.

Co-authored-by: linghengqian <20187731+linghengqian@users.noreply.github.com>
Copy link
Author

Copilot AI commented Jan 28, 2026

confirm.

Completed the package realignment to handler/ral/queryable, handler/ral/updatable, and handler/rul (sources, tests, SPI, reachability metadata). Commit: 1ea8e20.

Copilot AI changed the title Migrate eligible proxy DistSQL executors into new kernel/distsql module Align DistSQL handler package layout in kernel/distsql Jan 28, 2026
Copilot AI requested a review from linghengqian January 28, 2026 04:54
@linghengqian linghengqian requested a review from Copilot January 28, 2026 05:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates DistSQL handler executors from the proxy/backend/handler/distsql package to a new kernel/distsql/handler module, aligning with the final package structure for the ShardingSphere kernel architecture.

Changes:

  • Created new Maven module structure (kernel/distsql and kernel/distsql/handler)
  • Migrated RAL queryable/updatable and RUL handlers to new package namespace
  • Updated SPI registrations and native-image reachability metadata

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated no comments.

Show a summary per file
File Description
kernel/pom.xml Added distsql submodule to kernel parent
kernel/distsql/pom.xml New parent POM for distsql module
kernel/distsql/handler/pom.xml New handler module with appropriate dependencies
kernel/distsql/handler/src/main/java/**/*.java Migrated executor implementations with package updates
kernel/distsql/handler/src/test/java/**/*.java Migrated and simplified test files
kernel/distsql/handler/.../DistSQLUpdateExecutor New SPI registrations for migrated updatable executors
kernel/distsql/handler/.../DistSQLQueryExecutor New SPI registrations for migrated queryable executors
proxy/backend/core/pom.xml Added dependency on new distsql-handler module
proxy/backend/core/.../DistSQLUpdateExecutor Removed SPI registrations for migrated executors
proxy/backend/core/.../DistSQLQueryExecutor Removed SPI registrations for migrated executors
proxy/backend/core/src/test/java/**/*.java Deleted migrated test files
infra/reachability-metadata/.../reachability-metadata.json Updated class references for native-image compilation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +113 to +125
private HintValueContext copyHintValueContext(final HintValueContext hintValueContext) {
HintValueContext result = new HintValueContext();
result.getShardingDatabaseValues().putAll(hintValueContext.getShardingDatabaseValues());
result.getShardingTableValues().putAll(hintValueContext.getShardingTableValues());
result.getDisableAuditNames().addAll(hintValueContext.getDisableAuditNames());
result.setDataSourceName(hintValueContext.getDataSourceName());
result.setDatabaseShardingOnly(hintValueContext.isDatabaseShardingOnly());
result.setWriteRouteOnly(hintValueContext.isWriteRouteOnly());
result.setSkipSQLRewrite(hintValueContext.isSkipSQLRewrite());
result.setShadow(hintValueContext.isShadow());
return result;
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain why it is necessary to manually copy a new class.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new helper copies the HintValueContext so we can set skipMetadataValidate without mutating the shared context from the connection (avoids side effects on the original QueryContext). This keeps preview execution isolated. Commit: 1ea8e20.

Copilot AI requested a review from linghengqian January 28, 2026 05:45
@linghengqian linghengqian deleted the copilot/migrate-distsql-modules branch February 12, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants