FlyingZC commented on PR #39711:
URL: https://github.com/apache/shardingsphere/pull/39711#issuecomment-5523047436
### Result
[0/1127]
**Review Result: Mergeable**
PR [#39711](https://github.com/apache/shardingsphere/pull/39711)
completely implements Oracle DML error logging across the parser, statement
model, and binder
layers. No blocking code-correctness issues were found.
### Evidence
- `ErrorLoggingSegment` is defined in the Apache parser statement core and
retains the SQL range, error table, tag expression, and reject limit:
`parser/sql/
statement/core/src/main/java/org/apache/shardingsphere/sql/parser/statement/core/segment/dml/ErrorLoggingSegment.java:31`.
- INSERT, UPDATE, DELETE, and MERGE statements expose the model through
`Optional<ErrorLoggingSegment>` while preserving existing builder compatibility.
- The Oracle visitor constructs the segment for INSERT, UPDATE, DELETE,
and MERGE:
`parser/sql/engine/dialect/oracle/src/main/java/org/apache/shardingsphere/sql/
parser/engine/oracle/visitor/statement/type/OracleDMLStatementVisitor.java:287`.
- `visitMultiTableElement` preserves the table, columns, values,
insert-select clause, and error logging clause of every nested INSERT,
preventing the visitor
from dropping syntax already accepted by the grammar:
`OracleDMLStatementVisitor.java:602`.
- The Insert, Update, and Delete binders preserve error logging when
copying statements. MERGE currently has no dedicated copy binder, so
`DMLStatementBindEngine`
retains the original statement.
- Visitor tests cover all four DML statement types, table/tag/reject-limit
values, parameter indexes, and two multi-table elements. Parser IT covers
positive
segment assertions for all four statement types and verifies absence for
existing cases. Binder tests cover all three applicable copy paths.
- The model and clause placement are consistent with Oracle’s official
[INSERT](https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf/INSERT.html),
[UPDATE](https://docs.oracle.com/en/database/oracle/oracle-database/19/sqlrf/UPDATE.html),
[DELETE](https://docs.oracle.com/en/database/oracle/oracle-database/26/
sqlrf/DELETE.html), and
[MERGE](https://docs.oracle.com/en/database/oracle/oracle-database/26/sqlrf/MERGE.html)
documentation.
- Verification passed: 5 visitor tests, 13 binder tests, and 1,950 Oracle
Parser IT cases. File-scoped Spotless and Checkstyle checks passed. The full
Unit/IT
gate completed with `status=completed` and `failures_found=0`.
### Coverage
- Review type: Public PR Code Correctness Review.
- Public head: `4b3549ff8fcc033bf6b5a3e4bfb23fddcf6b4330`.
- Public base: `63a188eacaac9a5779b9ddda4e6f8af56e62546d`; merge base:
`5bf860b8f96b5b37f2abc61a985992a5c9ff66d8`.
- The GitHub `/files` response exactly matches the local triple-dot scope:
31 files, 1 commit, 504 additions, and 8 deletions.
- The review covered all statement-model, Oracle visitor, binder-copy,
visitor-test, binder-test, Parser IT JAXB/assertion, and SQL case resource
changes.
- Root-cause, blast-radius, tests/runtime, and final convergence passes
were completed. The final head, file-list hash, and discussion state remained
unchanged,
with no unresolved evidence gaps.
- Oracle is the target trunk dialect. The PR does not modify grammar or
introduce DM, GoldenDB, SphereEx, rewrite, or other downstream product code.
- This result covers code correctness only. CI was not queried or
evaluated, as required by Code Correctness Review mode.
--
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]
