strongduanmu commented on PR #39165:
URL: https://github.com/apache/shardingsphere/pull/39165#issuecomment-5000168842
**Review Result: Mergeable**
Reason: This is a code-scope-only result. CI was not reviewed by request;
within the reviewed production-code scope, no merge-blocking correctness,
compatibility, ownership, or test-adequacy issue remains.
### Evidence
- The PR changes only two encrypt rewrite production files:
-
`features/encrypt/core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/context/EncryptSQLRewriteContextDecorator.java`
-
`features/encrypt/core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/insert/EncryptInsertDefaultColumnsTokenGenerator.java`
- The decorator still gates encrypt rewrite by checking whether the
current statement context contains an encrypt table before adding encrypt token
generators.
- `EncryptInsertDefaultColumnsTokenGenerator` now resolves the
`EncryptTable` once before choosing the previous-token path or new-token path,
and both branches still call the same column rewrite logic.
- Existing tests cover the changed classes:
- `EncryptSQLRewriteContextDecoratorTest`
- `EncryptInsertDefaultColumnsTokenGeneratorTest`
### Review Details
- Review Focus: Code Correctness Review. CI not reviewed by request.
- Reviewed Scope: latest PR head
`c5299ed8f55c11c75f2e96afc39159f5efc6aa39`; base ref `master`, GitHub base SHA
`68b0b182ce2f97e33e1652b2c11377f04174c8c6`; local merge-base
`a096c1ab8811116b4b491d0c1a0224d395794253`. Local triple-dot file list matched
GitHub
`/pulls/39165/files`.
- Not Reviewed Scope: GitHub Actions, check-runs, workflow logs, pipeline,
and broader encrypt behavior outside the two changed files.
- Verification: PR metadata, GitHub file list, issue comments, PR reviews,
review threads, diff, changed code paths, and existing focused tests were
reviewed. Local Maven sentinel was attempted but did not complete successfully
because of local snapshot
dependency / stale ServiceLoader classpath state; this local environment
issue was not used as review evidence.
- Release Note / User Docs: Not required; this is an internal encrypt
rewrite refactor with no user-facing configuration, API, migration,
compatibility, or troubleshooting surface.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
