terrymanu commented on PR #39585: URL: https://github.com/apache/shardingsphere/pull/39585#issuecomment-5386603203
### Result **Review Result: Mergeable** This PR fills the only remaining syntax gap of issue #27017 in the smallest correct way: it adds `CHANGE DB_UNIQUE_NAME FROM identifier TO identifier` to the Oracle DAL `change` rule, fully reusing the field-less passthrough model established by #39581, with a regression parser IT case. No blocking issue was found. ### Evidence - **Scope verification**: PR head `8b80a8e2db3` (1 commit, 3 files, +4/-1), base `cfa4dfe7b50`. Local HEAD equals the public head and the local triple-dot file list matches the authoritative GitHub list 3/3 (verified by the inventory script). - **Official syntax evidence**: the Oracle 21c RMAN Reference CHANGE page (docs.oracle.com/en/database/oracle/oracle-database/21/rcmrf/CHANGE.html) defines `CHANGE DB_UNIQUE_NAME FROM db_unique_name TO db_unique_name;` and the PR case `BOSTON_A → BOSTON_B` matches that documentation and the issue sample. Supporting RMAN commands as Oracle DAL statements is established precedent (#39554 SET DBID/LIST/REPORT, #39581 CHANGE family). - **Grammar correctness**: the new alternative at `parser/sql/engine/dialect/oracle/src/main/antlr4/imports/oracle/DALStatement.g4:207` is added as the first alternative; its leading token `DB_UNIQUE_NAME` is disjoint from the four existing alternatives (BACKUPSET/DATAFILECOPY/COPY/BACKUP), so no shadowing or path change occurs. `CHANGE CATEGORY` inside `alterOutline` uses a different entry token and is unaffected. `changeOption` is intentionally not attached to this form, matching the official syntax. The value position uses `identifier`, same as the merged `forDbUniqueName : FOR DB_UNIQUE_NAME identifier` precedent (#39554). - **Reuse and blast radius**: the dispatch (`OracleStatement.g4:169 | change`), `visitChange`, the field-less `OracleChangeStatement`, and the assert/JAXB registration all pre-exist from #39581 and are unchanged in this PR, keeping the model consistent. Oracle is a trunk dialect with no branch dialects, so no diff markers are required; ANTLR generated artifacts are untracked build output with no baseline impact. - **Test validity**: the new `change_db_unique_name_oracle` case in `sql/supported/dal/change.xml` and `case/dal/change.xml` has a unique id and its expected shape matches the 8 existing change cases; removing the grammar branch would fail this case, which provides real regression protection. - **Local verification (sanitized)**: `./mvnw -pl test/it/parser -am -DskipITs -Dspotless.skip=true -Dtest=...oracle.InternalOracleParserIT ... test` → Tests run: 1940, Failures: 0, exit code 0; `./mvnw spotless:apply -Pcheck -T1C` and `./mvnw checkstyle:check -Pcheck -T1C` both BUILD SUCCESS (exit code 0). The verified content is byte-identical to commit `8b80a8e2db3`. ### Coverage - Reviewed head: `8b80a8e2db3`; requirements: PR description plus `Fixes #27017`. - Files: 3/3 accounted for (behavior clusters: `oracle-change-grammar` 1 file, `oracle-change-it-case` 2 files), no churn-only files; all three discovery lenses (root cause/behavior, blast radius/contracts, tests/runtime) completed, convergence pass found zero new candidates; no candidate passed the Finding Proof Gate as a blocker. - Dialect: Oracle (trunk dialect, no branch dialects); no shared code or other dialect touched. Official docs: Oracle 21c RMAN CHANGE page. Repository docs/baselines: no change required. - Scope notes: the remaining CHECK/CASE WHEN snippets in issue #27017 are expressions rather than statements, hence outside statement-parsing scope and not a PR gap; unrelated local uncommitted worktree changes (oracle select-special-function.xml) are not part of the PR head and were excluded from review scope. - Gaps: none. CI not reviewed (this result is code-scope only; GitHub Actions/checks were neither queried nor judged). -- 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]
