strongduanmu commented on PR #38675:
URL: https://github.com/apache/shardingsphere/pull/38675#issuecomment-4403168981
• Merge Verdict: Mergeable
Reviewed Scope:
kernel/sql-federation/compiler/src/main/java/.../SQLStatementCompilerEngine.java
latest PR diff, related cache loader path, JDBC/Proxy SQL federation entry
points, CI/E2E status.
Not Reviewed Scope: full SQL federation runtime scenario and full module
coverage.
Need Expert Review: No.
Review 建议：
This change is in the right direction. The previous debug log used
LoadingCache#get to check whether the execution plan cache existed, which could
trigger the cache loader and compile the SQL before the
real compile path. Using getIfPresent keeps the log check side-effect free
while preserving the existing behavior for the actual useCache branch.
CI and E2E have passed, and the change is limited to debug logging in
SQLStatementCompilerEngine. I do not see a blocking regression risk for this PR.
本地我也跑过：
./mvnw -pl kernel/sql-federation/compiler -am -DskipITs
-Dspotless.skip=true -Dtest=SQLStatementCompilerEngineTest
-Dsurefire.failIfNoSpecifiedTests=false test
--
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]
