terrymanu commented on PR #38877:
URL: https://github.com/apache/shardingsphere/pull/38877#issuecomment-4761203382

   ### Summary
   
   - **Merge Decision: Mergeable**
   - **Reason:** The PR narrowly adds the documented Hive Iceberg `OPTIMIZE 
TABLE ... REWRITE DATA` syntax, wires it into the Hive parser/visitor path, and 
includes focused parser IT coverage.
   
   ### Evidence
   
   - Apache Iceberg’s Hive compaction docs list `OPTIMIZE TABLE t REWRITE DATA` 
as a supported Hive 4 Iceberg compaction form: 
https://iceberg.apache.org/docs/latest/hive/#compaction.
   - 
`parser/sql/engine/dialect/hive/src/main/antlr4/imports/hive/DALStatement.g4:42`
 defines the new `optimizeTable` rule as `OPTIMIZE TABLE tableName REWRITE 
DATA`, and 
`parser/sql/engine/dialect/hive/src/main/antlr4/org/apache/shardingsphere/sql/parser/autogen/HiveStatement.g4:54`
 wires it into the Hive `execute` entry point.
   - 
`parser/sql/engine/dialect/hive/src/main/java/org/apache/shardingsphere/sql/parser/engine/hive/visitor/statement/type/HiveDALStatementVisitor.java:95`
 visits the new rule and returns an optimize-table statement with the parsed 
table segment.
   - `test/it/parser/src/main/resources/sql/supported/dal/optimize.xml:21` and 
`test/it/parser/src/main/resources/case/dal/optimize.xml:25` add direct Hive 
parser coverage for `OPTIMIZE TABLE t REWRITE DATA`.
   - The linked issue is broader Iceberg-related Hive syntax work, but this PR 
does not claim to close it; the reviewed implementation scope is the single 
documented OPTIMIZE compaction form.
   - No new public production type, public/shared API contract, cache/session 
state, dependency, distribution, or migration surface is introduced. Hive has 
no branch dialect family in the repository SQL parsing standards; MySQL/Doris 
optimize visitors were checked only as adjacent statement-class reuse.
   
   ### Review Details
   
   - **Reviewed Scope:** Latest PR head 
`ff4f310ed65009c1bc2db2769ba1286e715035f3`; base ref `apache/master` fetched at 
`c52991467cbe370a8785c3e8ac4c02fb36e5e66b`; local merge-base 
`5ee6bcfab90b32b0be2bdc2ba8b4a652127131c0`. Reviewed all 7 GitHub 
`/pulls/38877/files` entries, and the local triple-dot file list matched 
GitHub. Target dialect: Hive. Related trunk/branch dialects: none applicable 
for Hive. Official syntax source checked: Apache Iceberg Hive compaction docs.
   - **Not Reviewed Scope:** Full repository build, GitHub Actions/check-run 
status, live HiveServer2/Iceberg execution, and remaining Iceberg syntax under 
issue `#37074` outside this PR’s concrete OPTIMIZE compaction form.
   - **Verification:** `./mvnw -pl test/it/parser -am -DskipITs 
-Dspotless.skip=true -Djacoco.skip=true 
-Dtest=org.apache.shardingsphere.test.it.sql.parser.hive.InternalHiveParserIT 
-Dsurefire.failIfNoSpecifiedTests=false test` exited `0` with `Tests run: 738, 
Failures: 0, Errors: 0, Skipped: 0`. `./mvnw -pl 
parser/sql/engine/dialect/hive,test/it/parser -am -DskipTests -DskipITs 
-Dspotless.skip=true -Djacoco.skip=true checkstyle:check -Pcheck -T1C` exited 
`0`.
   - **Release Note / User Docs:** Release note is required for parser 
user-visible syntax support and is present at `RELEASE-NOTES.md:36`. Additional 
user docs are not required for this narrow parser acceptance because no 
configuration, migration, or usage behavior beyond the documented Hive/Iceberg 
syntax is introduced.


-- 
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]

Reply via email to