terrymanu commented on PR #39586:
URL: https://github.com/apache/shardingsphere/pull/39586#issuecomment-5386683538
### Result
**Review Result: Mergeable**
The PR now delivers complete first-class Doris BITOR support: lexer keyword
registration, a parse-rule extension, and a sql-case with assertion, all
verified green; the previously flagged unrelated Oracle commit is gone.
Optional polish only: the PR title and both commit messages still read "Add
Doris BITOR sql-case and assertion", which now understates the grammar work.
### Evidence
- Scope: head `7947eae57ae` on `terrymanu:dev`, merge-base `cfa4dfe7b50`, 2
commits, 4 files, +26/−1; the local branch content is identical to the PR head
and the local triple-dot file list matches the authoritative GitHub file list.
The prior blocker (unrelated Oracle RMAN commit `8b80a8e2db3`) is no longer
present in the PR.
- Grammar
(`parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/DorisKeyword.g4`):
new `BITOR` lexer keyword inside `// DORIS ADDED` markers, matching the
adjacent `BITXOR` pattern; longest-match keeps the existing `BIT` token
unaffected.
- Grammar
(`parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/BaseRule.g4`):
`bitwiseBinaryFunctionName` extended to `BITXOR | BITOR`, so `SELECT
BITOR(3,5)` parses through `specialFunction → bitwiseFunction` and the existing
`visitBitwiseFunction` produces a `FunctionSegment` with literal parameters;
`BITOR` is also added to `identifierKeywordsUnambiguous`, so `bitor` remains
usable as a column or table identifier — the same compatibility treatment
`BITXOR` already has.
- Semantics: the 2-argument form matches the official Doris function
reference (`BITOR(Integer, Integer)`, e.g. `select bitor(3,5)` returns 7,
branch-2.1 `bitor.md`).
- Tests
(`test/it/parser/src/main/resources/sql/supported/dml/select-special-function.xml`
and `case/dml/select-special-function.xml`): `select_bitor_doris` sql-case and
matching assertion with zero-based indices 7..16 for the function and 13/15 for
the literals, consistent with the `select_bitxor` sibling.
- Verification (local, sanitized): `InternalDorisParserIT` on this exact
tree — 1372 tests, 0 failures, 0 errors (exit 0), with the `select_bitor_doris
LITERAL Doris` parameter confirmed generated and executed; scoped
`spotless:apply` and `checkstyle:check` both passed (exit 0).
### Coverage
- Reviewed head: `7947eae57ae`; requirements: PR title/body ("Fixes
#31509"), issue #31509 BITOR task, official Doris BITOR documentation; all 4
authoritative files reviewed hunk-by-hunk; no public reviews or comments exist
yet.
- Behavior clusters: 1 — Doris BITOR first-class parsing support (grammar +
visitor reuse + regression case); no churn-only files.
- Discovery lenses completed: root cause and behavior (issue item satisfied;
old identifier-path parsing superseded deterministically by the keyword path),
blast radius and contracts (Doris is a MySQL branch dialect; only
Doris-specific grammar files with `DORIS ADDED` markers touched, MySQL trunk
untouched, no statement-model, SPI, or public API changes,
`bitor`-as-identifier compatibility preserved), tests/runtime/operations
(regression case executes and asserts the function segment; style checks pass).
- Unresolved gaps: none that could change the conclusion.
- Non-blocking note: consider retitling the PR (e.g. "Support parsing Doris
BITOR function") and using a distinct message for the grammar commit, since the
current title/messages describe only the test resources.
- CI scope: this is a Code Correctness Review of code scope only; GitHub
Actions and checks were not reviewed.
--
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]
