Support parsing Doris RECOVER syntax for database, table, and partition#38660
Open
daguimu wants to merge 1 commit into
Open
Support parsing Doris RECOVER syntax for database, table, and partition#38660daguimu wants to merge 1 commit into
daguimu wants to merge 1 commit into
Conversation
terrymanu
requested changes
May 28, 2026
terrymanu
left a comment
Member
There was a problem hiding this comment.
Decision
- Merge Verdict: Not Mergeable
- Reviewed Scope: Latest PR #38660 head
cc9d02cecebb04bce5ea7ccad8b433c35aee4e96; local merge-base75474b1aa85f9f2e1ad72b2482fadb9dabb18253; local diff file list matched GitHub/pulls/38660/fileswith 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
RECOVERstatement intoDorisStatement.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:
recoverPartitioncurrently ends atPARTITION partitionName NUMBER_? FROM tableName, andvisitRecoverPartitionnever setsnewNamefor partition recovery. The added SQL cases stop atRECOVER PARTITION p1 30003 FROM example_db.example_tbland 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_namefor 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.newNameforPARTITION, 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.
- Symptom:
Next Steps
- Add Doris grammar support for partition recovery rename and align the exact token order with the official Doris version being targeted.
- Update
DorisDALStatementVisitorso partition recovery carriesnewNamein 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 -T1Cand./mvnw checkstyle:check -Pcheck -T1C.
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 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Doris ships a
RECOVERadministration 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:Root Cause
DALStatement.g4for the Doris dialect has no rule forRECOVERoutside of XA transactions, so each form fails ANTLR parsing before any visitor runs.Fix
recover,recoverDatabase,recoverTable, andrecoverPartitionrules to the DorisDALStatement.g4, allowing the optional numeric id and the optionalAS new_nametail for database/table forms, andPARTITION name [id] FROM [db.]tablefor the partition form.executerule ofDorisStatement.g4.DorisRecoverStatement(object type, database/table segment, partition name, optional id, optional new name) and the matchingvisitRecoverlogic inDorisDALStatementVisitor.SQLVisitorRuleand the assert/test-case glue (DorisRecoverStatementAssert,DorisRecoverStatementTestCase,DorisDALStatementAssert,RootSQLParserTestCases).Tests Added
test/it/parser/src/main/resources/sql/supported/dal/recover.xmlcovering 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.xmlwith the matching assertions.InternalDorisParserITruns cleanly: 1250 tests pass after the change.Fixes
Refs #31503