strongduanmu commented on PR #39286:
URL: https://github.com/apache/shardingsphere/pull/39286#issuecomment-5140001224
• Review result: Mergeable
I reviewed PR #39286 against the latest head
`f98b2e7570db504de052e41b373ba58bee462fc4`, focusing on Oracle database object
DDL parsing correctness for `CREATE FUNCTION`, `CREATE TRIGGER`, and `CREATE
MATERIALIZED VIEW`.
The two previously identified edge cases are excluded from this review per
follow-up decision:
- System trigger `ENABLE` / `DISABLE` / `WHEN` handling.
- Trigger body `CALL schema.package.procedure(...)`.
Within the remaining scope, I did not find additional blocking correctness
issues.
Reviewed areas:
- Oracle PL/SQL grammar changes for function, trigger, compound trigger,
database trigger events, and call specifications.
- Oracle DDL visitor changes for trigger metadata extraction, function
inner SQL collection, procedure call capture, dynamic SQL expression capture,
and materialized view select extraction.
- Statement model changes for `CreateFunctionStatement`,
`CreateTriggerStatement`, and `CreateMaterializedViewStatement`.
- Parser IT additions and assertion wiring for trigger procedure-call
metadata.
- Oracle function cases for Java/C call specifications and aggregate
function syntax.
Positive checks:
- `CREATE FUNCTION ... PARALLEL_ENABLE AGGREGATE USING ...` is supported.
- `PIPELINED` function forms with `PARALLEL_ENABLE(PARTITION ... BY
HASH/VALUE ...)` and `ORDER/CLUSTER BY` streaming clauses parse successfully in
the checked order.
- Trigger body `CALL DBMS_OUTPUT.PUT_LINE(...)` is captured as a procedure
call.
- Trigger body local DML statements are collected into
`CreateTriggerStatement#getSqlStatements()`.
- Trigger table extraction works for ordinary DML triggers.
- The parser IT assertion path for `CreateTriggerStatement` is wired into
standard DDL assertions.
Residual follow-up items:
- Add parser IT coverage for system trigger `ENABLE`, `DISABLE`, and
`WHEN`.
- Add parser IT coverage for schema-qualified package routine calls in
trigger body.
- Consider extending trigger assertions beyond procedure-call names if
downstream consumers rely on trigger inner SQL statements, dynamic SQL
expressions, or body-end names.
--
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]
