terrymanu commented on PR #39521: URL: https://github.com/apache/shardingsphere/pull/39521#issuecomment-5344353635
### Result **Review Result: Mergeable** PR #39521 (head `95c319d661b`, base `7bf39a9c37b`, 13 files, +254/-0, "Fixes #31502") fully implements the two remaining gaps from the linked issue with no blocking findings. The BITMAP column data type is delivered via a `dataType` enumeration alternative plus `identifierKeywordsUnambiguous` compatibility entries; SHOW DATA SKEW is delivered via the new `showDataSkew` grammar rule, `DorisShowDataSkewStatement`, the `visitShowDataSkew` visitor, and the standard attribute set — each aligned point-by-point with established repository patterns (`STRING`/`ARRAY` type additions, `DorisShowQueryStatsStatement`). ### Evidence - Grammar matches the official syntax: `showDataSkew : SHOW DATA SKEW fromTable (PARTITION LP_ partitionName (COMMA_ partitionName)* RP_)?` (`parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/DALStatement.g4:306`) covers every form documented in the Doris manual, `SHOW DATA SKEW FROM [db.]table [PARTITION(p1, ...)]`; the BITMAP data type corresponds to the Doris type documentation linked from the issue. - Keyword-ization compatibility risk is mitigated: BITMAP and SKEW are both added to `identifierKeywordsUnambiguous` (`BaseRule.g4:157`, `BaseRule.g4:505`), so `bitmap`/`skew` remain usable as identifiers; the only rule in the whole Doris grammar consuming bare `IDENTIFIER_` is the MySQL-inherited `KILL` rule (`DALStatement.g4:687`), an exposure identical to every previously merged keyword addition (BITXOR, STRING, ARRAY) and not a new regression class introduced by this PR. - Dispatch and routing chain holds: for `SHOW DATA SKEW` the root context is `ShowContext`, which hits the existing `SHOW` entry and routes to `DorisDALStatementVisitor`; `visitShowDataSkew` (`DorisDALStatementVisitor.java:1160`) carries no dead null-guard for the mandatory `fromTable`, uses `isEmpty()` for the partition list, and the four statement attributes mirror the merged `DorisShowQueryStatsStatement`. - Tests protect real behavior: four SHOW cases cover all grammar branches (no partition / db-qualified / single partition / multiple partitions), the BITMAP case covers both a plain column and a `BITMAP_UNION` aggregate column, and the assertions verify extracted table/partition/type segments rather than parse success alone. - Sanitized local verification on the same head: module builds exit 0; `InternalDorisParserIT` runs 1299 tests with 0 failures and 0 errors (including the 5 new cases and the full Doris regression); `spotless:apply` and `checkstyle:check` both exit 0. ### Coverage - Reviewed head: `95c319d661b`; the GitHub changed-file list matches the local triple-dot file list exactly across all 13 files. Requirement source: the PR description `Fixes #31502` and the issue case list; the PR has no prior comments or reviews. - Three behavior clusters (BITMAP type / SHOW DATA SKEW statement / IT wiring); all 13 files map to a cluster with no churn-only files; all three discovery lenses (root cause and behavior, blast radius and contracts, tests-runtime-operations) completed; 12 candidates examined, 0 passed the Finding Proof Gate. - Dialect family: Doris is a branch dialect of the MySQL trunk; MySQL trunk and the MariaDB sibling were checked — all changes are confined to `imports/doris/` and test infrastructure, with no shared grammar file touched. Official documentation used: the Doris 4.x SHOW-DATA-SKEW manual page and the BITMAP type documentation linked from the issue. Repository docs and release notes need no update per the convention for dialect parser PRs (#39201, #37552). - Unresolved gaps: none. This result is code-scope only; CI was not queried or reviewed, and the repository's required CI process still applies before merging. -- 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]
