terrymanu commented on PR #39520:
URL: https://github.com/apache/shardingsphere/pull/39520#issuecomment-5344146496
### Result
**Review Result: Mergeable**
Test-only change that correctly registers two new Doris `ANALYZE TABLE`
parser IT cases for syntax forms already supported by the grammar merged in
#39518; both forms match official Doris documentation and the full Doris parser
IT suite passes locally with the new cases included.
### Evidence
- Scope: single commit `7c84978bf52` on base `c678e88cddc` (master); the
GitHub changed-file list (2 files, +4/-0) matches the local triple-dot diff. No
production code, no public contracts, no other dialects touched.
- Grammar support:
`parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/DALStatement.g4:384-393`
— `analyzeTable` accepts `(LP_ columnNames RP_)?` and `analyzeOption*`, and
`analyzeOption` includes `WITH SYNC` and `WITH SAMPLE (PERCENT | ROWS)
numberLiterals`, so both new SQL texts parse; `SYNC`, `PERCENT`, `SAMPLE`,
`ANALYZE` tokens all exist in `DorisKeyword.g4`.
- Official syntax: the Apache Doris 2.1 SQL manual (ANALYZE statement)
documents `ANALYZE TABLE <table_name> [(<column_name> [, ...])] [[WITH SYNC]
[WITH SAMPLE {PERCENT | ROWS} <sample_rate>]]`, covering both `ANALYZE TABLE
lineitem WITH SYNC` and `ANALYZE TABLE lineitem (l_orderkey, l_quantity) WITH
SAMPLE PERCENT 10`.
- Pattern consistency: the new `<common sql-case-id="..."/>` entries at
`test/it/parser/src/main/resources/case/dal/analyze-table.xml:23-24` are
parse-only cases, identical in shape to the three existing Doris cases added in
#39518; the visitor maps `AnalyzeTableContext` to `AnalyzeTableStatement`
carrying the table list, and no assert path consumes AnalyzeTable segments, so
empty assertion content is the established correct shape.
- Integrity: case IDs `analyze_table_with_sync` and
`analyze_table_with_columns_and_sample_percent` are unique across all parser IT
resources (no collision with Oracle/MySQL analyze IDs in
`sql/supported/ddl/analyze.xml`); both XML files are well-formed with ASF
license headers intact and `db-types` scoped to Doris.
- Local verification: `./mvnw -pl test/it/parser -am verify
-Dit.test=InternalDorisParserIT ...` exited 0 with `Tests run: 1294, Failures:
0, Errors: 0, Skipped: 0` for `InternalDorisParserIT`; the packaged test
resources loaded by the run contain both new entries, so both new cases
executed and passed. An initial non-`-am` run had 351 errors, all
`ClassNotFoundException: DorisCreateWorkloadGroupStatement` caused by stale
local snapshot artifacts predating #39518; the current-source `-am` rebuild
eliminated them, so that failure is environmental and not attributable to this
PR.
### Coverage
- Reviewed head: `7c84978bf529bd5c5ca307013d533d521bea768a` (PR #39520, base
master `c678e88cddc`); both authoritative files accounted for via the GitHub
file list and a local inventory script; no churn files; no prior reviews or
comments to reconcile.
- Behavior clusters: 1 — "extend Doris ANALYZE TABLE parse-only IT coverage"
covering both changed files.
- Discovery lenses completed: root cause and behavior (coverage for grammar
from #39518), blast radius and contracts (case-ID uniqueness, Doris-only
scoping, no API/SPI/grammar changes), tests and runtime (green local IT run
including the new cases).
- Dialects: Doris (target, MySQL-family derived grammar untouched); MySQL,
PostgreSQL, and Oracle analyze cases live in separate files and were checked
for ID collisions only, as the PR does not modify shared code.
- Official documentation used: Apache Doris 2.1 SQL manual, ANALYZE
statement page. Repo docs/examples: not required for a test-only change.
- Unresolved gaps: none for code correctness. CI was not reviewed; this
result is code-scope only.
--
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]
