Skip to content

Support parsing Doris RECOVER syntax for database, table, and partition#38660

Open
daguimu wants to merge 1 commit into
apache:masterfrom
daguimu:fix/doris-recover-31503
Open

Support parsing Doris RECOVER syntax for database, table, and partition#38660
daguimu wants to merge 1 commit into
apache:masterfrom
daguimu:fix/doris-recover-31503

Conversation

@daguimu

@daguimu daguimu commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Problem

Doris ships a RECOVER administration statement that brings a dropped database, table, or partition back from the recycle bin, optionally with the original object id and/or a renamed target. None of the variants listed in #31503 are accepted by the current Doris parser:

RECOVER DATABASE example_db
RECOVER DATABASE example_db 10001
RECOVER DATABASE example_db AS new_db
RECOVER TABLE example_db.example_tbl
RECOVER TABLE example_db.example_tbl 20002
RECOVER TABLE example_db.example_tbl 20002 AS new_tbl
RECOVER PARTITION p1 FROM example_tbl
RECOVER PARTITION p1 FROM example_db.example_tbl
RECOVER PARTITION p1 30003 FROM example_db.example_tbl

Root Cause

DALStatement.g4 for the Doris dialect has no rule for RECOVER outside of XA transactions, so each form fails ANTLR parsing before any visitor runs.

Fix

  • Add recover, recoverDatabase, recoverTable, and recoverPartition rules to the Doris DALStatement.g4, allowing the optional numeric id and the optional AS new_name tail for database/table forms, and PARTITION name [id] FROM [db.]table for the partition form.
  • Wire the new alternative into the top-level execute rule of DorisStatement.g4.
  • Introduce DorisRecoverStatement (object type, database/table segment, partition name, optional id, optional new name) and the matching visitRecover logic in DorisDALStatementVisitor.
  • Register the SPI entry in SQLVisitorRule and the assert/test-case glue (DorisRecoverStatementAssert, DorisRecoverStatementTestCase, DorisDALStatementAssert, RootSQLParserTestCases).

Tests Added

  • test/it/parser/src/main/resources/sql/supported/dal/recover.xml covering the nine Doris syntaxes from Support parsing Doris sql #31503 plus a partition variant without the database-qualified table.
  • test/it/parser/src/main/resources/case/dal/recover.xml with the matching assertions.
  • InternalDorisParserIT runs cleanly: 1250 tests pass after the change.

Fixes

Refs #31503

@terrymanu terrymanu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decision

  • Merge Verdict: Not Mergeable
  • Reviewed Scope: Latest PR #38660 head cc9d02cecebb04bce5ea7ccad8b433c35aee4e96; local merge-base 75474b1aa85f9f2e1ad72b2482fadb9dabb18253; local diff file list matched GitHub /pulls/38660/files with 11 files. Reviewed Doris parser grammar, visitor, new statement type, parser IT assert/case XML, and related MySQL/Doris RECOVER paths.
  • Not Reviewed Scope: GitHub Actions/CI status, full Maven verification, actual Doris FE execution, and unrelated parser dialects beyond the Doris/MySQL family impact check.
  • Need Expert Review: Doris SQL parser maintainer review is recommended after the syntax gap below is fixed; no dedicated security or concurrency review is needed.

Positive Feedback

  • The PR is in the right direction: it wires the Doris RECOVER statement into DorisStatement.g4, adds a Doris-specific statement object, and adds parser IT fixtures for the basic database/table/partition recovery forms listed in the issue.

Major Issues

  • [Major] Documented partition rename syntax is still rejected (parser/sql/engine/dialect/doris/src/main/antlr4/imports/doris/DALStatement.g4:898)
    • Symptom: recoverPartition currently ends at PARTITION partitionName NUMBER_? FROM tableName, and visitRecoverPartition never sets newName for partition recovery. The added SQL cases stop at RECOVER PARTITION p1 30003 FROM example_db.example_tbl and do not cover partition rename (test/it/parser/src/main/resources/sql/supported/dal/recover.xml:27).
    • Risk: The root cause is only partially fixed. Doris official RECOVER docs state that recovered metadata can be renamed and include new_partition_name for partitions, for example in the Doris 3.x RECOVER docs and Doris 4.x RECOVER docs. Valid adjacent syntax such as partition recovery with a new partition name remains unparsed, so users still hit the original parser failure for a documented RECOVER form.
    • Action: Please extend the Doris grammar and visitor to support the documented partition rename form for the target Doris version, populate DorisRecoverStatement.newName for PARTITION, and add one-to-one parser IT SQL/case assertions for the partition rename path. Please also add adjacent coverage for documented optional combinations such as database recovery with both id and new name, and table recovery with new name but without id.

Next Steps

  • Add Doris grammar support for partition recovery rename and align the exact token order with the official Doris version being targeted.
  • Update DorisDALStatementVisitor so partition recovery carries newName in the same statement model as database/table recovery.
  • Add parser IT cases and assertions for partition rename, plus missing optional combinations around id and AS.
  • Re-run the scoped parser verification with dependencies, for example: ./mvnw -pl test/it/parser -am -DskipITs -Dspotless.skip=true -Dtest=org.apache.shardingsphere.test.it.sql.parser.doris.InternalDorisParserIT -Dsurefire.failIfNoSpecifiedTests=false test.
  • Run the repository formatting/style gates before the next review: ./mvnw spotless:apply -Pcheck -T1C and ./mvnw checkstyle:check -Pcheck -T1C.

@terrymanu

Copy link
Copy Markdown
Member

Hi @daguimu, thanks again for working on this Doris parser improvement.

This PR has had no response or update for a long time after the change request. The current implementation is in the right direction, but it is still not ready to merge because the documented Doris RECOVER PARTITION ... AS <new_partition_name> syntax is not supported yet, and the related parser test coverage is still incomplete.

To keep the review queue clear, we will close this PR soon due to inactivity. You are very welcome to reopen it or submit a new PR later with the missing Doris RECOVER syntax support and matching parser IT assertions.

Thanks again for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants