terrymanu commented on issue #31482: URL: https://github.com/apache/shardingsphere/issues/31482#issuecomment-5344885983
### Result **Review Result: Mergeable** PR head `1f6311216e82` (local HEAD matches the GitHub head) contains three behavior clusters, all verified against official documentation and existing repository patterns, with no blocking issues found. One non-blocking suggestion: the PR title and description only cover WINDOW_FUNNEL (#31482), but the change also includes the BITMAP data type and the SHOW DATA SKEW statement fixing #31502 (that issue is already closed); consider mentioning the second part in the PR description for post-squash-merge traceability. ### Evidence **Cluster 1: OVER clause after regular function calls (WINDOW_FUNNEL, #31482)** - `parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/BaseRule.g4:1316` appends `overClause?` to `completeRegularFunction` inside the existing `// DORIS CHANGED` block, consistent with the PostgreSQL `funcApplication ... overClause?` precedent and Doris's own `aggregationFunction ... overClause?`; zero trunk (MySQL) file changes and branch diff markers fully preserved. - Syntax basis: Doris 3.x official documentation defines WINDOW_FUNNEL as an aggregate function `WINDOW_FUNNEL(window, mode, timestamp, event_1...)` that can be combined with OVER as a window function (https://doris.apache.org/docs/3.x/sql-manual/sql-functions/aggregate-functions/window-funnel/). The legacy two-phase syntax from the issue links is the deprecated Doris 1.2 form; excluding it is the right call. - `visitCompleteRegularFunction` needs no change; the `FunctionSegment` text automatically includes OVER, and the assertion shape matches the existing `select_window_partition_order_doris` case. **Cluster 2: SHOW DATA SKEW statement (#31502)** - The new `showDataSkew` rule in `DALStatement.g4` matches the official syntax item by item: `SHOW DATA SKEW FROM [<db_name>.]<table_name> [PARTITION (<partition_name> [, ...])]`; official example 2 is exactly the comma-separated multi-partition form (https://doris.apache.org/docs/2.1/sql-manual/sql-statements/table-and-view/data-and-status-management/SHOW-DATA-SKEW/). - The four attributes on `DorisShowDataSkewStatement`, the coexistence of the manual `Optional<SimpleTableSegment> getTable()` with Lombok, and the `(FromTableSegment) visit(ctx.fromTable())` cast in `visitShowDataSkew` all follow the established patterns of the sibling `DorisShowQueryStatsStatement`/`visitShowIndex`. - No adjacent-behavior regression: the `showData` alternative is unaffected (`SHOW DATA`, `SHOW DATA FROM t`, and `... ORDER BY col` all verified as passing). **Cluster 3: BITMAP data type (#31502)** - `dataType` gains `dataTypeName = BITMAP`; the new `BITMAP`/`SKEW` lexer rules are also added to `identifierKeywordsUnambiguous` (with the `BITMAP_UNION` precedent), all with DORIS markers; `BIT` vs `BITMAP` resolves safely by longest match. **Tests and runtime verification (measured on the PR head)** - 8 new sql-cases (4 `show_data_skew_*`, 1 `create_table_with_bitmap_type_doris`, 2 `select_window_funnel*`) plus the `select_length` extension to Doris; all case ids are unique, and assertion indices were re-verified character by character (including owner qualification, multiple partitions, and binary-operation parameters). - `./mvnw -pl parser/sql/engine/dialect/doris -DskipITs -Dspotless.skip=true test` exit 0 (6/6). - `./mvnw -pl test/it/parser -Dtest=...doris.InternalDorisParserIT test` exit 0 (1302/1302, including all new cases). - `./mvnw -pl test/it/parser -Dtest=...mysql.InternalMySQLParserIT test` exit 0 (1901/1901, shared-resource regression). - Direct parse probes: all four SHOW DATA SKEW forms (db-qualified, single/multiple partitions), the BITMAP CREATE TABLE statement, and the existing SHOW DATA statements pass. ### Coverage - Reviewed candidate: PR #39522 head `1f6311216e8285da7123ed46de0692b637bb64f7`, merge-base `7bf39a9c37b`; the local triple-dot file list exactly matches the 18 files from GitHub `/pulls/39522/files`, with no related dirty working-tree changes. - Behavior-cluster mapping: all 18 files are mapped into the three clusters above (`BaseRule.g4` belongs to clusters 1/3 and `DorisKeyword.g4` to clusters 2/3), with no churn-only files; the completion-gate ledger marked 18/18 files and passed validation. - All three discovery lenses (root cause and behavior, blast radius and contracts, tests and runtime) plus the final convergence pass completed, with zero new findings in the convergence pass. - Dialect family check: Doris is a branch dialect of the MySQL trunk; this PR touches no trunk files; the MariaDB branch is unaffected; the MySQL IT regression passed. - Official documentation basis: the two Doris documentation links above; linked issues #31482 (open, whose 4 remaining checklist items are all covered by this PR: WINDOW_FUNNEL ×2 via grammar plus cases, LENGTH ×2 via case extension, INSERT via existing parsing and existing case coverage) and #31502 (closed). - Unresolved gaps: none. CI was neither queried nor reviewed; this conclusion is a code-scope readiness judgment 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]
