terrymanu commented on PR #39518:
URL: https://github.com/apache/shardingsphere/pull/39518#issuecomment-5343621022
### Result
**Review Result: Mergeable**
The single blocking issue from the previous review round — the three new
Doris keywords (`PERCENT`, `SAMPLE`, `WORKLOAD`) not being registered as usable
identifiers — is Fixed on the latest head
`97a255aa7c74e6e1dda1f9542bf8603b08da8c77`: commit `97a255aa` registers all
three tokens in `identifierKeywordsUnambiguous` with `// DORIS ADDED` markers
at alphabetical positions and adds a dedicated regression case. The full-scope
convergence pass over all 15 changed files found no new independent candidates,
and scoped verification on a tree identical to the public head is green.
### Evidence
- Fix verification:
`parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/BaseRule.g4` now
contains `| PERCENT` (between `PAUSE` and `PHASE`), `| SAMPLE` (between `RTREE`
and `SCHEDULE`), and `| WORKLOAD` (between `WORK` and `WRAPPER`) inside
`identifierKeywordsUnambiguous`, each wrapped with `// DORIS ADDED BEGIN/END`
per the `INSTR`/`PLAN` precedent; Doris SQL using these words as unquoted
identifiers parses again, matching the same-family precedents (#37588
registered `PERCENT`, #37561 registered `INVERTED`).
- Regression protection:
`test/it/parser/src/main/resources/sql/supported/dal/create.xml` and
`case/dal/create.xml` add `create_workload_group_with_keyword_group_name`
(`CREATE WORKLOAD GROUP sample PROPERTIES("max_cpu_percent"="10%")`) asserting
the keyword-derived group name and property, which fails if the registration is
removed.
- Sanitized verification on a tree identical to the public head (local
`HEAD` equals `97a255aa7c74`, parent equals PR base `master@cd741fd4b4`,
working tree clean): `./mvnw -pl
parser/sql/engine/core,parser/sql/engine/dialect/doris,parser/sql/statement/dialect/doris,test/it/parser
-Dtest=...InternalDorisParserIT -Dsurefire.failIfNoSpecifiedTests=false
-Dspotless.skip=true test` → exit 0 (`Tests run: 1292, Failures: 0, Errors:
0`); `./mvnw spotless:apply -Pcheck -T1C` → exit 0; `./mvnw checkstyle:check
-Pcheck -T1C` → exit 0.
- Scope integrity: the GitHub changed-file list (15 files, +227/−1) matches
the local merge-base diff one-to-one; the fix commit touches only the three fix
files, leaving the previously reviewed 14-file content unchanged.
### Coverage
- Reviewed head `97a255aa7c74e6e1dda1f9542bf8603b08da8c77` (2 commits, base
`master@cd741fd4b4`); authoritative requirements are issue #31486's remaining
checklist items plus the prior round's required change; all 15 files accounted
for with no churn-only files.
- Behavior clusters: C1 CREATE WORKLOAD GROUP (grammar, keywords, identifier
registration, dispatch, visitor, statement class, assert/JAXB/resources); C2
ANALYZE TABLE extension with `WITH SAMPLE PERCENT|ROWS` (grammar plus cases);
C3 keyword-identifier compatibility fix spanning `DorisKeyword.g4` and
`BaseRule.g4`. All three discovery lenses and the convergence pass completed
with zero findings passing the proof gate.
- Dialect family: Doris is a MySQL trunk branch; MySQL/MariaDB grammar files
are untouched, branch diff markers (`// DORIS CHANGED`, `// DORIS ADDED`)
follow `CODE_OF_CONDUCT.md`; the identifier-list widening only affects Doris
and does not change token streams or the explicit keyword matching in
`analyzeOption`/`createWorkloadGroup`.
- Official documentation used: `doris-website`
`docs/sql-manual/sql-statements/cluster-management/compute-management/CREATE-WORKLOAD-GROUP.md`
and `docs/sql-manual/sql-statements/statistics/ANALYZE.md`; quoted group names
are accepted via the `DOUBLE_QUOTED_TEXT` branch of `identifier`.
- Known non-blocking scope boundary (not required by #31486): `ANALYZE
DATABASE`, `WITH HOT VALUE`, and the cloud-mode `FOR <compute_group>` clause
remain unsupported.
- Unresolved gaps: none. This result is code-scope only; CI was not
reviewed. The PR has no public reviews or comments yet.
--
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]
