terrymanu commented on PR #38824: URL: https://github.com/apache/shardingsphere/pull/38824#issuecomment-4637908454
### Summary - **Merge Decision: Mergeable** - **Reason:** The latest change is a scoped test-only refactor, the removed prepared-statement cache integration scenarios are covered by owner-level tests, and scoped verification passed locally. ### Evidence - The PR changes only `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/StandardDatabaseProxyConnectorTest.java`, replacing the deep Firebird prepared-statement fixture with a thin `PREPARED_STATEMENT` execute smoke test at `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/StandardDatabaseProxyConnectorTest.java:480`. - The removed cache behavior is still covered in the owning components: same-key reuse at `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/jdbc/statement/JDBCBackendStatementTest.java:106`, invalidation at `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/jdbc/statement/JDBCBackendStatementTest.java:167`, cache lifecycle at `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/session/ConnectionSessionTest.java:177`, and cache close/held-transaction cleanup at `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/ProxyDatabaseConnectionManagerTest.java:273` and `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/ProxyDatabaseConnectionManagerTest.java:532`. - Adjacent prepared-statement paths remain covered by driver-type selection at `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/DatabaseProxyConnectorFactoryTest.java:114`, callback selection at `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/jdbc/executor/callback/ProxyJDBCExecutorCallbackFactoryTest.java:41`, and prepared callback execution at `proxy/backend/core/src/test/java/org/apache/shardingsphere/proxy/backend/connector/jdbc/executor/callback/impl/ProxyPreparedStatementExecutorCallbackTest.java:40`. - No production code, public API/SPI, parser behavior, metadata/routing logic, dependency, distribution, diagnostics, or user-facing behavior is changed, so release notes and user documentation are not required. - No substantive unrelated changes were found in the authoritative PR file scope. ### Review Details - **Reviewed Scope:** Latest PR head `42d58f007618ea148c9b15f0dd7d13a47d2b779a`; base and local merge-base `6741d909c2953ca663678e9997ff8921f7c90838`; reviewed one modified file under `proxy/backend/core`. Local triple-dot file list matched GitHub `/pulls/38824/files`. - **Not Reviewed Scope:** GitHub Actions/check-runs, full-repository runtime integration, and live Proxy/JDBC database execution were not reviewed because this PR is test-only and CI status is outside this review workflow. - **Verification:** `git fetch apache master refs/pull/38824/head:refs/remotes/apache/pr/38824` exited 0; `git diff --name-status $(git merge-base apache/master apache/pr/38824)..apache/pr/38824` exited 0 and matched GitHub files; `./mvnw -pl proxy/backend/core -am -DskipITs -Dspotless.skip=true -Dtest=StandardDatabaseProxyConnectorTest,JDBCBackendStatementTest,PreparedStatementCacheContextTest,ConnectionSessionTest,ProxyDatabaseConnectionManagerTest -Dsurefire.failIfNoSpecifiedTests=false test` exited 0; `./mvnw -pl proxy/backend/core -am -DskipTests -DskipITs checkstyle:check -Pcheck -T1C` exited 0; `./mvnw -pl proxy/backend/core -am -DskipTests -DskipITs spotless:check -Pcheck -T1C` exited 0. - **Release Note / User Docs:** Not required; this is a test-only refactor with no user-visible behavior 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
