terrymanu commented on PR #38301:
URL: https://github.com/apache/shardingsphere/pull/38301#issuecomment-4012484302
Merge Verdict: Not Mergeable
Reviewed Scope
- Latest PR head 9d034c3008f0e9a745711f92cf589b92bc4c7ebf (7 changed
files).
- Hive parser grammar/visitor changes and parser IT SQL/case resources.
- Related assert/case classes:
- AlterTableStatementTestCase.java
- AlterTableStatementAssert.java
Not Reviewed Scope
- No local full Maven regression run.
- No deep per-job log review for every CI job.
Need Expert Review
- in: SQL parse maintainers should confirm long-term model design for the
new executeSegment.
### Major Issues
1. Blocking: test evidence is not mapped 1:1 to the fix points
- Root-cause path is touched (grammar + visitor + new segment), but tests
only verify parse acceptance, not semantic fields.
- Current AlterTable parser IT does not assert executeType / snapshotId
semantics.
- Risk: semantic regression can pass CI unnoticed.
- Please add dedicated expected/assert support for:
- executeType = CHERRY-PICK
- snapshotId value and index range
- large snapshot-id boundary case
2. Blocking: PR is currently not mergeable (mergeable_state=dirty)
- The branch is in conflict with master.
- Please rebase/merge master, rerun CI, then request re-review on the new
latest commit.
--
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]
