terrymanu commented on PR #40231: URL: https://github.com/apache/shardingsphere/pull/40231#issuecomment-6259865545
### Result Review Result: Mergeable The local candidate meets the minimal fix requirements, with no blocking issues. ### Evidence - Both new regression tests failed against the original implementation. After the fix, all 216 loader tests and 5 strategy tests passed. - Raw JaCoCo counters (covered/missed; CLASS, LINE, BRANCH): outer class 1/0, 38/0, 42/0; ScriptReader 1/0, 326/4, 340/12. Both branches of the new condition are covered. ConditionalBlock and ScriptStatement contain only Lombok-generated behavior and have no counters of their own. No additional numeric coverage target was imposed. - The module's test and jacoco:report goals completed with exit code 0. Full commands and logs are retained in the verification record. - The full scanner still exits with code 1: the existing naming diagnostic is unchanged, the provider produces 21 actual rows, and the production-path diagnostic comes from the test-only scope check. Applicability was checked; the full scan was not reported as clean. ### Coverage - PR-backed local candidate: public and local HEAD are 93fbdb695fbbf231893600a6a91b8208f548f86c. The complete review baseline is e81c7c9aa5b49f6c1939eb62ec5c9f6c70e2491d, covering 2 files and 16 changed hunks. The uncommitted local delta consists of one production condition and two test-data rows; the public PR has not yet been updated. - P: test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/external/loader/template/dialect/PostgreSQLExternalTestParameterLoadTemplate.java - T: test/it/parser/src/test/java/org/apache/shardingsphere/test/it/sql/parser/external/loader/template/dialect/PostgreSQLExternalTestParameterLoadTemplateTest.java - Requirements come from the user's minimal plan, official psql semantics, and the existing PostgreSQL corpus-loading contract. Unknown conditions, nesting, COPY, echo, and malformed-input replay retain their existing behavior. No SQL syntax, expression evaluator, API, or additional dialect support was introduced. - Root cause and behavior, impact and contracts, and tests and runtime were fully reviewed. Five rounds of independent paired assessments and report reconciliation were completed. All historical candidates were resolved, and the final two rounds agreed. Session validation exited with code 0. No unresolved evidence gaps remain, and no exemption was used. - Spotless and Checkstyle both checked P and T. Combined command: ./mvnw -pl test/it/parser -DspotlessFiles="$SPOTLESS_FILES" -Dcheckstyle.includes="$CHECKSTYLE_FILES" -Dcheckstyle.includeResources=false -Dcheckstyle.includeTestResources=false spotless:check checkstyle:check -Pcheck -T1C, exit code 0. The variables represent the two exact absolute-path regexes and Java-source-root-relative paths, respectively. Checkstyle reported 0 errors and 18 warnings. - RAT is not applicable: no files were added, and no license headers or build configuration were changed. This conclusion covers code correctness only; CI was not assessed. Local style verification is complete. -- 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]
