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/06/22 17:10:25 UTC
[PR] Refactor IndexReviseEngine [shardingsphere]
terrymanu opened a new pull request, #38888:
URL: https://github.com/apache/shardingsphere/pull/38888
Revise #38449
--
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 IndexReviseEngine [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #38888:
URL: https://github.com/apache/shardingsphere/pull/38888#issuecomment-4770954741
### Summary
**Review Result: Mergeable**
Reason: No merge-blocking issue was found in the latest PR version. The change removes the unused convenience overloads from `IndexReviseEngine`, keeps the production path on the explicit 4-argument revision context, and the scoped tests pass.
### Evidence
- `infra/common/src/main/java/org/apache/shardingsphere/infra/metadata/database/schema/reviser/index/IndexReviseEngine.java:51` now exposes only the explicit revision method with `originalTableMetaDataList` and `schemaMetaDataRevisionCandidateTableMetaDataList`.
- `infra/common/src/main/java/org/apache/shardingsphere/infra/metadata/database/schema/reviser/table/TableMetaDataReviseEngine.java:77` still passes both metadata collections through the production metadata revision path, so the index-name recovery context added around #38449 is preserved.
- Repository reference search found no remaining in-repo callers of the removed 2-argument or 3-argument overloads.
- Updated tests cover both the direct engine path and the shared sharding index revision path:
- `IndexReviseEngineTest`
- `ShardingIndexReviserTest`
### Review Details
- Reviewed Scope: PR #38888 latest head `dd0883324f4eaa33b415d1c1b5faf2fd0653b9cd`, base / merge-base `cf0af7586852de48cc4534b6b142dea3623678a2`. GitHub `/pulls/38888/files` matched the local triple-dot diff. Reviewed changed files:
- `infra/common/src/main/java/org/apache/shardingsphere/infra/metadata/database/schema/reviser/index/IndexReviseEngine.java`
- `infra/common/src/test/java/org/apache/shardingsphere/infra/metadata/database/schema/reviser/index/IndexReviseEngineTest.java`
- `features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/metadata/reviser/index/ShardingIndexReviserTest.java`
- Not Reviewed Scope: full E2E/proxy runtime behavior, unchanged modules, and full logs for public CI jobs still in progress.
- Verification:
- `git diff --name-status cf0af7586852de48cc4534b6b142dea3623678a2..dd0883324f4eaa33b415d1c1b5faf2fd0653b9cd`: exit 0, matched the 3 GitHub changed files.
- `./mvnw -pl infra/common,features/sharding/core -am -DskipITs -Dsurefire.failIfNoSpecifiedTests=false -Dtest=IndexReviseEngineTest,ShardingIndexReviserTest test`: exit 0, `IndexReviseEngineTest` 4 tests passed, `ShardingIndexReviserTest` 10 tests passed, `BUILD SUCCESS`.
- Public checks observed for the latest head: Spotless, CheckStyle, and License completed successfully; several broader CI/E2E jobs were still in progress and were not required for this refactor-level judgment.
- Release Note / User Docs: not required. This is an internal metadata revision refactor with no user-facing configuration, SQL behavior, migration step, or documentation change.
--
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 IndexReviseEngine to use explicit revision context [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #38888:
URL: https://github.com/apache/shardingsphere/pull/38888
--
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