terrymanu commented on PR #40126:
URL: https://github.com/apache/shardingsphere/pull/40126#issuecomment-6094304093
### Result
**Review Result: Mergeable**
The current local PR1 candidate preserves the approved static SQL loading
and error-filtering capabilities. The PostgreSQL class migration and Iterator
simplification have no remaining blocking issues. This conclusion applies only
to the reviewed code scope.
### Evidence
- The current candidate passes 89 tests, with zero failures, errors, or
skipped tests. Coverage is 196/196 SUT-owned branches and 222/223 lines; the
remaining line is Lombok-generated exception code.
- Spotless and Checkstyle explicitly cover all three current Java files and
pass. Checkstyle reports zero errors and eight warnings. RAT passes, and the
verification results match the current source hashes.
- The new class loads through the current IT annotation and
`getConstructor().newInstance()`, with no legacy class remaining in the current
build. The 2,071 outputs from eight frozen sample pairs are byte-for-byte
identical to those before simplification. SELECT and CLUSTER parser and visitor
smoke checks pass.
- Two predetermined AB/BA comparisons between the original 81-line
implementation and the current implementation show that the differences in min,
median, and p95 remain within the original frozen 9.60 ms threshold. The
performance gate passes. The current median remains approximately 5 ms higher
per eight files. Earlier 24.35/25.31 ms tail spikes are retained and remain
unattributed; no speedup or claim that every run passed the threshold is made.
- The existing DO grammar entry-point gap is outside scope. Loading a
complete DO statement does not establish parser support, and the historical CI
job cannot be promised to pass.
### Coverage
- This is an independent local candidate. The original baseline remains
`6a600fd40c0a866bc291a0960d65e54b1d7d1941`. All four changed paths were
reviewed:
- Deleted legacy class:
`/Users/zhangliang/.codex/worktrees/postgresql-loader-pr1/shardingsphere/test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/external/loader/template/type/StandardExternalTestParameterLoadTemplate.java`
-
[PostgreSQLExternalTestParameterLoadTemplate.java](/Users/zhangliang/.codex/worktrees/postgresql-loader-pr1/shardingsphere/test/it/parser/src/main/java/org/apache/shardingsphere/test/it/sql/parser/external/loader/template/dialect/PostgreSQLExternalTestParameterLoadTemplate.java:54)
-
[PostgreSQLExternalTestParameterLoadTemplateTest.java](/Users/zhangliang/.codex/worktrees/postgresql-loader-pr1/shardingsphere/test/it/parser/src/test/java/org/apache/shardingsphere/test/it/sql/parser/external/loader/template/dialect/PostgreSQLExternalTestParameterLoadTemplateTest.java:38)
-
[ExternalPostgreSQLParserIT.java](/Users/zhangliang/.codex/worktrees/postgresql-loader-pr1/shardingsphere/test/it/parser/src/test/java/org/apache/shardingsphere/test/it/sql/parser/postgresql/ExternalPostgreSQLParserIT.java:25)
- The approved scope covers source-text fidelity, statement boundaries, COPY
payload isolation, unambiguous error attribution, class migration, and the
reflection contract. It does not expand into a complete psql interpreter or
additional grammar support.
- Three review layers were completed: root cause and behavior, impact scope
and contracts, and tests and runtime. Functional, compatibility, testing,
performance, concurrency, security and operations, dependency, and
generated-artifact risks were checked. The independent defensive-code audit
found no issues.
- The inventory covers all four paths. Review ledger convergence validation
passed, and the ledger was cleaned up. No evidence gap affects the conclusion.
The scanner CLI's limitation with deleted paths was addressed using its
existing functions and an independent manual scope audit; the original CLI is
not claimed to have passed in full. Seven Git-ignored IDE files were preserved
and excluded from the task candidate.
- Local formatting verification is complete. The full external IT suite was
not rerun, and GitHub CI was not reviewed.
--
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]
