terrymanu commented on PR #39587: URL: https://github.com/apache/shardingsphere/pull/39587#issuecomment-5386765624
### Result **Review Result: Mergeable** This PR adds Doris `ADMIN CANCEL REBALANCE DISK` parsing support and Doris sql-cases for YEARWEEK/RTRIM. The grammar matches the official Doris definition exactly, the change is purely additive with no regression signal, and all verification passed (code-scope only; CI was not reviewed). ### Evidence - **Grammar correctness**: `parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/DALStatement.g4:517` defines `adminCancelRebalanceDisk : ADMIN CANCEL REBALANCE DISK (ON LP_ string_ (COMMA_ string_)* RP_)?`, which matches the official Apache Doris master grammar `fe/fe-sql-parser/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4` rule `adminCancelRebalanceDisk` token by token (verified this session against a download of apache/doris master). - **Complete behavior chain**: `DorisStatement.g4:162` alternative → `DorisDALStatementVisitor.java:1331` `visitAdminCancelRebalanceDisk` (extracts backends with `SQLUtils.getExactlyValue` quote stripping, consistent with the sibling `visitAdminRebalanceDisk`) → `DorisAdminCancelRebalanceDiskStatement` → `SQLVisitorRule.java:612` enum entry. This exactly mirrors the merged precedents #39510 (ADMIN REBALANCE DISK) and #39583 (ADMIN CANCEL REPAIR). - **Test validity**: 3 new cases cover all 3 grammar branches (no ON clause, multiple backends, single backend) and assert backend content and order; `InternalDorisParserIT` NPEs on any sql-case without a loaded expected assertion, so the full pass proves the `admin-cancel-rebalance-disk` JAXB element registration and `DorisDALStatementAssert` dispatch actually executed. - **No regression**: the change is purely additive (none of the 12 files modifies an existing rule or enum entry); MySQL trunk grammar files and the MariaDB dialect are untouched; all 1372 pre-existing Doris cases plus 5 new cases pass (1377 total) in `InternalDorisParserIT`. - **Local verification** (run on a working tree identical to the PR head content): parser three-module `-am install` exit 0; `InternalDorisParserIT` `Tests run: 1377, Failures: 0, Errors: 0` exit 0; `spotless:apply -Pcheck` exit 0; `checkstyle:check -Pcheck` exit 0. ### Coverage - **Reviewed candidate**: PR head `0a0592b` (object SHA identical to local dev HEAD), base/merge-base `701077d`; the GitHub authoritative file list and the local triple-dot diff match 12/12 (+214/−2), single commit, no prior review feedback to re-evaluate. - **Requirements mapping**: the PR body declares Fixes #31507; 9 of the issue's 10 SQL items were already supported upstream, and this PR completes the last item `ADMIN CANCEL REBALANCE DISK` plus regression cases for YEARWEEK/RTRIM, matching the issue goal with no out-of-scope changes. - **Behavior clusters**: C1 new-statement parsing chain (11 files) and C2 db-types extension of two existing cases (1 file); every file is mapped; no churn-only files. - **Dialect family check**: Doris is a branch dialect of MySQL; the MySQL trunk `DALStatement.g4` is unmodified, sibling dialect MariaDB has no shared file affected; `CANCEL`/`REBALANCE`/`DISK` are all pre-existing tokens with no lexer conflict. - **Finding classification**: the only candidate found across the three discovery lenses (root cause and behavior / blast radius and contracts / tests, runtime, and operations) — the new rule lacking `// DORIS ADDED BEGIN/END` markers — failed the Finding Proof Gate as non-blocking: the adjacent `adminRebalanceDisk`/`adminRepair`/`adminCancelRepair` rules merged via #39510/#39583 are likewise unmarked, so this PR follows the merged local precedent; if a reviewer later requests markers per CODE_OF_CONDUCT, it is a two-comment change. - **Gaps**: no unresolved evidence or coverage gap. This conclusion is code-scope readiness (Code Correctness Review); GitHub Actions/CI state was neither queried nor awaited. -- 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]
