You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "FlyingZC (via GitHub)" <gi...@apache.org> on 2026/04/20 06:06:02 UTC

[PR] Revert "Fix incorrect routing when irrelevant sharding conditions are… [shardingsphere]

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

   … present (#38527)"
   
   This reverts commit dcbbaf9d
   
   Fixes #ISSUSE_ID.
   
   Changes proposed in this pull request:
     -
   
   ---
   
   Before committing this PR, I'm sure that I have checked the following options:
   - [ ] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
   - [ ] I have self-reviewed the commit code.
   - [ ] I have (or in comment I request) added corresponding labels for the pull request.
   - [ ] I have passed maven check locally : `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
   - [ ] I have made corresponding changes to the documentation.
   - [ ] I have added corresponding unit tests for my changes.
   - [ ] I have updated the Release Notes of the current development version. For more details, see [Update Release Note](https://shardingsphere.apache.org/community/en/involved/contribute/contributor/)
   


-- 
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] Revert "Fix incorrect routing when irrelevant sharding conditions are… [shardingsphere]

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

   **No blocking findings on the latest head of #38639.
   
     Decision
     Merge Verdict: Mergeable
   
     Reviewed Scope: RELEASE-NOTES.md, ShardingStandardRouteEngine.java, ShardingRouteEngineFixtureBuilder.java, ShardingStandardRouteEngineTest.java, SubqueryRouteTest.java,
     ShardingRouteAssert.java, shared caller ShardingComplexRouteEngine.java, PR metadata/checks for #38639, prior context from #38527.
   
     Not Reviewed Scope: full end-to-end logs for every GitHub matrix job, broader routing code outside the standard/complex engines, and any future replacement fix for #38456.
   
     Need Expert Review: No additional specialist review beyond normal sharding-route maintainer review.
   
     Basis
     This PR is a clean rollback of #38527, and the code changes match that intent. The production diff removes the added relevance-filtering and fallback logic from the shared standard
     routing path, plus removes the test scaffolding that existed only to support that change. The second commit only completes the revert in ShardingRouteAssert.java; I do not see unrelated
     scope creep.
   
     I specifically checked the shared-path blast radius. ShardingComplexRouteEngine still delegates into ShardingStandardRouteEngine, so I treated this as a shared routing-path review rather
     than a single-test revert. The revert restores the pre-#38527 behavior instead of introducing a new third state, which lowers semantic risk. I also checked the config-adjacent path: hint-
     based routing remains on the preexisting path and is still covered by existing tests.
   
     Verification
     Local:
   
     - ./mvnw -pl features/sharding/core -am -DskipITs -Dspotless.skip=true -Dtest=ShardingStandardRouteEngineTest,SubqueryRouteTest,ShardingComplexRouteEngineTest
       -Dsurefire.failIfNoSpecifiedTests=false test -> 0
     - ./mvnw -pl features/sharding/core -am -Pcheck -DskipTests checkstyle:check spotless:check -> 0
   
     GitHub:
   
     - PR API reports changed_files=6, commits=2, no extra churn.
     - Commit check-runs for 0a38ae689bfe87d8ea7e39bbd052de6bc32f7e2a show no failed checks in the returned set; the visible JDBC/Proxy MySQL/PostgreSQL smoke and stage-2 E2E jobs are green.
   
     Residual Risk
     This revert is mergeable as a rollback, but it intentionally stops trying to fix #38456. In other words, it is safe as a rollback PR, not as a root-cause fix for the original symptom. A
     follow-up PR still needs to repair #38456 with production-path regression coverage.**


-- 
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] Revert "Fix incorrect routing when irrelevant sharding conditions are… [shardingsphere]

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


-- 
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