terrymanu commented on PR #39523: URL: https://github.com/apache/shardingsphere/pull/39523#issuecomment-5345202553
### Result **Review Result: Mergeable** No blocking issues found within code scope. The PR fully covers the two remaining unchecked cases of linked issue #31489 (item 4, `array_first` lambda; item 6, the type-name list), every grammar addition is backed by official Doris documentation, and the full Doris parser regression passes. ### Evidence - **Candidate consistency**: PR head `716ea437fe5` equals local `dev` HEAD; the 6 files from GitHub `/pulls/39523/files` exactly match the local triple-dot file list (base `2dc62b91`), +199/-0, with no scope deviation. - **Lambda grammar** (`parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/BaseRule.g4:1045`, `BaseRule.g4:1055`): `(identifier | (identifier, ...)) -> expr` reuses the existing `JSON_SEPARATOR` token with no lexer change. Official basis: doris-website `sql-manual/sql-functions/scalar-functions/array-functions/array-first.md` (the `x -> x >= 3` and `(x, y) -> x > y` examples, available since Doris 2.0.0). Adjacent behavior of the pre-existing JSON-path rule `identifier -> string_` (`BaseRule.g4:1042`) is unchanged, verified by the full IT suite. - **Type extensions** (`DorisKeyword.g4:552`, `DorisKeyword.g4:1356`, `BaseRule.g4:1448`, `BaseRule.g4:1274`): `ARRAY LT_ dataType GT_` plus LARGEINT/DATEV2/DATETIMEV2 in both `dataType` and `castType`. Official basis: doris-website `sql-manual/basic-element/sql-data-types/semi-structured/ARRAY.md` (`ARRAY<VARCHAR(10)>`) and the type-name list scraped in issue #31489; the digit suffix `'2'` follows the FLOAT4 precedent in the same file. - **Compatibility**: all three new keywords are added to `identifierKeywordsUnambiguous` (`BaseRule.g4:210`, `BaseRule.g4:307`, following the BITMAP precedent), and `SELECT largeint, datev2 FROM t` parses, so same-named columns and tables keep working. - **Test validity**: the 6 new IT cases (single/multi-parameter lambda, lambda with empty array, `ARRAY<VARCHAR(10)>` DDL, the three-type DDL, and CAST with `DATETIMEV2(3)` precision) all failed to parse before the change (reproduced during analysis smoke runs) and pass after it; the full `InternalDorisParserIT` run of 1308 cases has 0 failures (including regression of all existing JSON, array-literal, and SQL_BLOCK_RULE cases); `spotless:apply` and `checkstyle:check` both exited 0. - **Contracts and boundaries**: no SPI, visitor, statement-model, or public API changes — parse-only support; the lambda subtree falls into the existing `CommonExpressionSegment` fallback with no downstream binder/rewriter impact. ### Coverage - **Reviewed scope**: head `716ea437fe5` (1 commit, 6 files, +199/-0) against base `master@2dc62b91`; 3 behavior clusters (lambda grammar, type-system extension, test-resource pairing); all 6 files mapped with no churn-only files; all three discovery lenses (root cause and behavior, blast radius and contracts, tests and runtime) completed with every candidate passed through the Finding Proof Gate. - **Dialect family**: Doris is a branch dialect of the MySQL trunk; this PR touches only the Doris-specific grammar module, with zero changes to MySQL trunk or MariaDB branch files, so neither dialect is affected. - **Known boundaries (non-blocking)**: space-free nesting `ARRAY<ARRAY<INT>>` is not supported because `>>` is lexed as the single `SIGNED_RIGHT_SHIFT_` token (the spaced form parses), and `castType` has no ARRAY branch. Both fall outside the case list of issue #31489 and can be tracked as follow-up issues. - **CI**: this is a Code Correctness Review; GitHub Actions state was not queried, awaited, or judged, so CI coverage is outside this conclusion. -- 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]
