terrymanu commented on PR #39589:
URL: https://github.com/apache/shardingsphere/pull/39589#issuecomment-5387182525
### Result
**Review Result: Mergeable**
This PR implements the remaining CREATE TYPE BODY capability of issue #27005
exactly along the official Oracle syntax, composed entirely from building
blocks already proven in PLSQL.g4 by CREATE PACKAGE BODY / CREATE FUNCTION /
CREATE TRIGGER, with correct ordering in the execute dispatch. No blocking
issue was found.
### Evidence
- **Scope verification**: PR head `be003d7abd0` (1 commit, 11 files,
+200/-0), base `965f3a95440`. Local HEAD equals the public head; the local
triple-dot file list matches the authoritative GitHub list 11/11 (inventory
script); no PR comments yet.
- **Official syntax evidence**: the Oracle 21c PL/SQL Language Reference
CREATE TYPE BODY statement
(docs.oracle.com/en/database/oracle/oracle-database/21/lnpls/CREATE-TYPE-BODY-statement.html)
and the SQL Reference CREATE TYPE BODY page (…/21/sqlrf/CREATE-TYPE-BODY.html)
define the member kinds: `[MEMBER|STATIC] FUNCTION/PROCEDURE` bodies,
`CONSTRUCTOR FUNCTION … RETURN SELF AS RESULT`, and `MAP|ORDER MEMBER FUNCTION`
bodies, each shaped as `heading {IS|AS} [declare_section] body`. The PR's
`typeBodyElement` covers all four kinds; both pages confirm the one-or-more
cardinality of `typeBodyElement+`.
- **Composition correctness**:
`parser/sql/engine/dialect/oracle/src/main/antlr4/imports/oracle/PLSQL.g4:118`
adds `createTypeBody`/`plsqlTypeBodySource`/`typeBodyElement`, reusing the
existing `body` (with `END identifier? SEMI_?` multi-END disambiguation),
`declareSection`, `parameterDeclaration`, and `returnDateType`, which are
already exercised in production by `createPackage` (`BODY
plsqlPackageBodySource`, PLSQL.g4:89-108) and
`plsqlProcedureSource`/`plsqlFunctionSource` (:118-141).
- **Dispatch correctness**: `createTypeBody` is inserted before `createType`
in `execute` (OracleStatement.g4), because `typeName` under `plsqlTypeSource`
could otherwise consume the `BODY` token; the ordering makes prediction
deterministic. No other `CREATE` statement shares the `CREATE … TYPE BODY`
prefix.
- **Model and contracts**: the new `OracleCreateTypeBodyStatement` extends
`DDLStatement` with a `typeName`, following the lightweight dialect-statement
family (`OracleChangeStatement`, `OracleListStatement`); `visitCreateTypeBody`
maps `typeName(0)`, which stays the leading name even when a trailing `END`
name is present. No existing statement, visitor, or contract is modified;
`CREATE_TYPE_BODY` is registered as a DDL visitor rule.
- **Test validity**: 4 IT cases cover all four member kinds: the #27005
sample (member functions with IF/ELSIF/ELSE), the official rational example
(MAP member + member procedure with `declareSection` and `END` with name), the
rectangle example (CONSTRUCTOR, MEMBER, STATIC), and an ORDER member case.
Removing the grammar branch fails all four; the JAXB registration mirrors the
`create-package` precedent and the case ids are unique.
- **Local verification (sanitized)**: `./mvnw -pl test/it/parser -am
-DskipITs -Dspotless.skip=true -Dtest=...oracle.InternalOracleParserIT ...
test` → Tests run: 1944, Failures: 0, exit code 0 (1940 pre-existing cases + 4
new); `./mvnw spotless:apply -Pcheck -T1C` and `./mvnw checkstyle:check -Pcheck
-T1C` both BUILD SUCCESS (exit code 0). The verified content is byte-identical
to commit `be003d7abd0`.
### Coverage
- Reviewed head: `be003d7abd0`; requirements: PR description plus `Fixes
#27005`.
- Files: 11/11 accounted for (behavior clusters: `oracle-type-body-grammar`
2 files, `oracle-type-body-wiring` 2, `oracle-type-body-model` 1,
`oracle-type-body-it` 6), no churn-only files; all three discovery lenses
completed, convergence pass found zero new candidates; no candidate passed the
Finding Proof Gate as a blocker.
- Dialect: Oracle (trunk dialect, no branch dialects); no shared code or
other dialect touched. Official docs: Oracle 21c lnpls and sqlrf CREATE TYPE
BODY pages. Repository docs/baselines: no change required (ANTLR generated
artifacts are untracked).
- Known scope boundaries, consistent with the issue-scoped parser family
precedent (#39554/#39581) and follow-up-able: `MEMBER|STATIC` is optional where
the official grammar requires it for plain subprograms, `EDITIONABLE` is
supported only in the family-convention position (before `TYPE BODY`, not after
the type name as in the official grammar), and the constructor's
`FINAL`/`INSTANTIABLE` markers are each accepted at most once where the
official grammar allows both. None of these rejects any form exercised by the
linked issue or the IT cases.
- Gaps: none. CI not reviewed (this result is code-scope only; GitHub
Actions/checks were neither queried nor judged).
--
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]
