terrymanu commented on PR #39571: URL: https://github.com/apache/shardingsphere/pull/39571#issuecomment-5384029950
### Result **Review Result: Mergeable** Scoped to the last commit only per explicit request: `cdb16f0b` (Add parser IT cases for Doris ARRAY_CUM_SUM function, 2 test-resource files, +68 lines). The commit is pure regression-test coverage with no production behavior change; assertion indices are empirically verified, the new cases execute and pair correctly, and no candidate passed the blocking-finding gate. ### Evidence - **Commit scope**: The authoritative GitHub file list from `/pulls/39571/commits` and `/repos/terrymanu/shardingsphere/commits/cdb16f0b` shows exactly `test/it/parser/src/main/resources/sql/supported/dml/select-special-function.xml` (+3) and `test/it/parser/src/main/resources/case/dml/select-special-function.xml` (+65), matching local `git show --stat cdb16f0b`; no unrelated changes are carried in. - **Assertion correctness**: On a local HEAD identical to this commit, `./mvnw -pl test/it/parser ... -Dtest=...doris.InternalDorisParserIT test` exited 0 with Tests run: 1341, Failures: 0. A negative control on byte-identical content (deliberately corrupting the stop-index of `select_array_cum_sum_empty_array` and rerunning) exited 1 and named that sql-case-id, proving the new cases are loaded and their assertions bind. - **Case-id pairing and uniqueness**: `select_array_cum_sum`, `select_array_cum_sum_empty_array`, and `select_array_cum_sum_from_table` each appear exactly once on both the sql/supported and case sides, with no conflict anywhere in the repository. - **Syntax and parsing path**: `ARRAY_CUM_SUM` parses through the generic `regularFunctionName ... | identifier` branch in `parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/BaseRule.g4`, with array arguments via `arrayExpression` (introduced by #38248). The three cases cover the non-empty array literal, the empty-array `LBT_ RBT_` branch, and the projection+FROM combination, mirroring the cosine_distance (#38248) and SPLIT_BY_STRING family precedents in the same files. `db-types="Doris"` restricts loading to InternalDorisParserIT, so the MySQL trunk and other dialects are unaffected. - **Gates**: `./mvnw spotless:apply -Pcheck -T1C` and `./mvnw checkstyle:check -Pcheck -T1C` both exited 0 (BUILD SUCCESS) on content byte-identical to this commit. ### Coverage - **Review scope**: The last commit `cdb16f0b` of PR #39571 (terrymanu:dev → apache:master), narrowed per explicit user request. The PR's two Oracle commits (`87d6a35e`, `e16f63a72ea`, touching oracle BaseRule.g4/PLSQL.g4 and 5 test-resource files) are outside this review. - **Behavior cluster**: A single cluster, "Doris ARRAY_CUM_SUM parser IT regression coverage"; both files map to it and there are no churn-only files. - **Discovery**: All three lenses (root cause/behavior, blast radius/contracts, tests/runtime) were walked; candidates (wrong indices, case-id collision, cross-dialect impact, coverage gap) were each disproven through the Finding Proof Gate, leaving no unresolved evidence gap. - **Dialect family**: Doris is a branch dialect of the MySQL trunk; this commit touches no shared grammar or visitor, so no trunk or sibling-dialect check is required (`db-types` isolation verified). - **Official documentation**: Doris array-functions documentation (the `Array<T> array_cum_sum(Array<T>)` usage listed in issue #31503); this commit adds no new syntax-support claim, only protection for already-supported syntax. - **CI**: This is a Code Correctness Review; GitHub Actions were not queried, awaited, or reported. External factors behind `mergeable_state: blocked` are outside this verdict. -- 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]
