terrymanu commented on PR #39510:
URL: https://github.com/apache/shardingsphere/pull/39510#issuecomment-5336603671
### Result
**Review Result: Mergeable**
PR #39510 (head `1ea7106f61b`, 1 commit, 15 files, +260/−2) correctly adds
Doris `ADMIN REBALANCE DISK` parsing and fixes the Doris `CREATE TABLE` clause
order plus multi-column/multi-model key clauses for CTAS. Both syntaxes match
the official Doris documentation, all parser ITs pass, and no blocking issue
survived the proof gate.
### Evidence
**Cluster A — `ADMIN REBALANCE DISK` (new statement)**
- Grammar
`parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/DALStatement.g4:478`
defines `ADMIN REBALANCE DISK (ON LP_ string_ (COMMA_ string_)* RP_)?`,
exactly matching the official Doris syntax `ADMIN REBALANCE DISK [ON
("host:port", ...)]` (doris.apache.org ADMIN-REBALANCE-DISK page, linked from
issue #31496). The rule shape is identical to the existing `adminCleanTrash`
rule at `DALStatement.g4:442`.
- `SQLVisitorRule.java:604` registers `ADMIN_REBALANCE_DISK` as a DAL rule;
`DorisDALStatementVisitor.java` `visitAdminRebalanceDisk` and the new
`DorisAdminRebalanceDiskStatement` (backends collection) are line-for-line
consistent with the maintained `AdminCleanTrash` precedent, including
`SQLUtils.getExactlyValue` quote stripping.
- Test wiring is complete and conventional: JAXB case type registered in
`RootSQLParserTestCases.java`, assert chained in
`DorisDALStatementAssert.java`, three SQL cases (plain, two backends, single
backend) with backend-content assertions.
**Cluster B — `CREATE TABLE` clause order and key clause**
- The reorder in `DDLStatement.g4:39` to `definitions → options → key →
comment → partition → distribution → properties → AS SELECT → START
TRANSACTION` matches the official Doris CREATE TABLE / CTAS clause order
(doris.apache.org CREATE-TABLE-AS-SELECT page); the old order placed `DUPLICATE
KEY` after `PARTITION BY`/`AS SELECT`, which cannot parse real Doris CTAS with
a key clause — this is the actual bug being fixed.
- `duplicatekeyClause` widening to `(DUPLICATE | UNIQUE | AGGREGATE) KEY LP_
columnNames RP_` covers all three Doris data models; `UNIQUE`, `AGGREGATE`
tokens already existed, and `columnNames` is the existing shared rule. No other
rule references `duplicatekeyClause`, so no hidden consumer breaks.
- Regression probe: every pre-existing Doris `CREATE TABLE` case (`ENGINE →
DUPLICATE KEY → DISTRIBUTED → PROPERTIES`, `PARTITION → DISTRIBUTED`, CTAS
without key) parses identically under the new order; full IT suite confirms.
- Issue-scope probe: the exact SQL from issue #31496, `CREATE TABLE
t_user(dt, id, name) ENGINE=OLAP UNIQUE KEY(dt, id) ... AS SELECT ...` with
bare column names, parses and visits into a `CreateTableStatement` with 3
column definitions and the select statement present (bare names parse via a
pre-existing optional branch in `dataType`, not introduced by this PR).
**Verification (local, at head `1ea7106f61b`; local HEAD equals PR head,
merge-base equals base `38527d1b84f`, local file list matches the GitHub list
of 15 files)**
- `./mvnw -pl test/it/parser verify ...` → exit 0: 10354 tests across all
dialect ITs, 0 failures, 0 errors; Doris report fresh with 1271 tests, 0
failures, including the new cases.
- Standalone parse probes against the PR-built Doris parser: issue-exact
bare-name CTAS PASS, both new PR cases PASS, `ADMIN REBALANCE DISK` with and
without `ON (...)` PASS, real-order `DUPLICATE KEY → PARTITION BY` PASS;
`SELECT rebalance FROM t` now fails (new keyword shadows the bare identifier) —
consistent with the repository-wide keyword-token convention (`TRASH`,
`REBUILD`, etc.), non-blocking.
- `./mvnw -Pcheck spotless:check checkstyle:check` on the four changed
modules → exit 0.
### Coverage
Reviewed head: `1ea7106f61b365bce6fc86442b65bdb17c04e0b9` against base
`38527d1b84f` (master). All 15 authoritative GitHub files were mapped to the
two behavior clusters above (grammar×4, visitor rule, visitor, statement, test
infra×4, resources×4) plus the shared `SQLVisitorRule` enum addition; no
churn-only files. Discovery lenses completed: root cause and behavior
(official-syntax comparison plus parse probes for new, adjacent, and old
orderings), blast radius and contracts (Doris-local grammar changes; shared
change is a purely additive enum constant; all-dialect IT run proves no
cross-dialect impact; no public API/SPI changes), tests and runtime (IT + style
gates green; asserts exercise backends content and CTAS projection indexes).
Target dialect Doris; related trunk (MySQL) grammar untouched and unaffected.
Official Doris documentation pages used: ADMIN-REBALANCE-DISK and
CREATE-TABLE-AS-SELECT. Unresolved gaps: none that could change the blocker
set. Non-blocking notes:
(1) `Fixes #31496` will auto-close the issue, which still lists unrelated
unchecked cases (L2_DISTANCE, REGEXP, RTRIM, CONCAT, bitmap_to_string) —
intended workflow is presumably a follow-up reopen or separate PRs; (2) the new
`REBALANCE` keyword makes an unquoted identifier `rebalance` unparseable,
matching existing keyword-addition convention. This result is code-scope only;
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]
