fix: support SUBPARTITION TEMPLATE for OceanBase composite partitioning#6630
fix: support SUBPARTITION TEMPLATE for OceanBase composite partitioning#6630daguimu wants to merge 1 commit intoalibaba:masterfrom
Conversation
Add SUBPARTITION TEMPLATE parsing to MySQL parser (used by OceanBase). Previously only SUBPARTITION OPTIONS was recognized after the subpartition-by clause, causing SUBPARTITION TEMPLATE to fail with "expect OPTIONS, actual IDENTIFIER TEMPLATE". The template contains subpartition definitions with VALUES LESS THAN or VALUES IN clauses that get applied to each partition in composite partitioning. Fixes alibaba#6160
50b7dab to
5a73b72
Compare
| for (; ; ) { | ||
| acceptIdentifier("SUBPARTITION"); | ||
| SQLSubPartition subPartition = new SQLSubPartition(); | ||
| subPartition.setName(this.exprParser.name()); | ||
| SQLPartitionValue values = this.exprParser.parsePartitionValues(); | ||
| if (values != null) { | ||
| subPartition.setValues(values); | ||
| } | ||
| subPartition.setParent(subPartitionByClause); | ||
| subPartitionByClause.getSubPartitionTemplate().add(subPartition); | ||
| if (lexer.token() == Token.COMMA) { | ||
| lexer.nextToken(); | ||
| continue; |
There was a problem hiding this comment.
[Suggestion] Template subpartitions are manually constructed with only name + VALUES, skipping per-subpartition properties (TABLESPACE, COMMENT, ENGINE, DATA DIRECTORY, etc.) that this.exprParser.parseSubPartition() handles. The Oracle parser's equivalent code correctly calls parseSubPartition().
Suggested fix: Replace the manual construction with a call to the existing parser method:
acceptIdentifier("SUBPARTITION");
SQLSubPartition subPartition = this.exprParser.parseSubPartition();
SQLPartitionValue values = this.exprParser.parsePartitionValues();
if (values != null) {
subPartition.setValues(values);
}
subPartition.setParent(subPartitionByClause);
subPartitionByClause.getSubPartitionTemplate().add(subPartition);| List<SQLStatement> stmtList = parser.parseStatementList(); | ||
| assertEquals(1, stmtList.size()); | ||
| } | ||
| } |
There was a problem hiding this comment.
[Suggestion] All 4 test cases assert stmtList.size() == 1 but never verify subPartitionTemplate contents (names, values, count). Neighboring tests (Issue5078, Issue6102) use round-trip verification via SQLParseAssertUtil.assertParseSql().
Suggested fix: Add AST-level assertions (after fixing output visitors for round-trip support):
SQLCreateTableStatement stmt = (SQLCreateTableStatement) stmtList.get(0);
SQLSubPartitionBy spBy = stmt.getPartitionBy().getSubPartitionBy();
List<SQLSubPartition> template = spBy.getSubPartitionTemplate();
assertEquals(3, template.size());
assertEquals("mp0", template.get(0).getName().getSimpleName());Or use SQLParseAssertUtil.assertParseSql(sql, dbType) for round-trip validation once output visitors are fixed.
wenshao
left a comment
There was a problem hiding this comment.
Review: fix: support SUBPARTITION TEMPLATE for OceanBase composite partitioning
Parsing logic is correct and all tests pass (49 OceanBase test classes, 0 failures). The PR successfully adds SUBPARTITION TEMPLATE parsing. However, the parsed template data has limited end-to-end functionality due to gaps in the existing AST infrastructure that this PR exposes.
Findings
1. accept0() in all SQLSubPartitionBy subclasses skips subPartitionTemplate [Suggestion]
The root cause: none of the accept0() implementations in SQLSubPartitionByHash, SQLSubPartitionByRange, SQLSubPartitionByList, MySqlSubPartitionByKey, MySqlSubPartitionByList, or MySqlSubPartitionByValue visit subPartitionTemplate. This makes template subpartitions invisible to ALL visitors — schema analysis, validation, statistics, and custom AST traversal. Fix: add acceptChild(visitor, subPartitionTemplate) to each accept0().
2. Output visitors silently drop SUBPARTITION TEMPLATE for all sub-partition types [Suggestion]
Only SQLASTOutputVisitor.visit(SQLSubPartitionByList) renders the template. visit(SQLSubPartitionByRange), visit(SQLSubPartitionByHash), and the MySQL-specific overrides (MySqlSubPartitionByList, MySqlSubPartitionByKey) all silently drop it. This means round-trip parse→output produces incorrect DDL that is missing the template. Fix: add template output blocks mirroring the existing visit(SQLSubPartitionByList) pattern.
3. Manual SQLSubPartition construction skips properties [Suggestion] (inline comment posted)
The new code manually constructs SQLSubPartition with only name + VALUES, while this.exprParser.parseSubPartition() handles TABLESPACE, COMMENT, ENGINE, etc. The Oracle parser's equivalent code calls parseSubPartition().
4. Tests only verify parse success, not AST structure [Suggestion] (inline comment posted)
All 4 test cases assert stmtList.size() == 1 without checking subPartitionTemplate contents. Neighboring tests use round-trip verification.
5. clone() for SQLSubPartitionByHash/SQLSubPartitionByRange loses template [Possibly, needs human review]
These clone() methods don't call super.cloneTo(), so cloning an AST with template subpartitions silently drops them.
Verdict: Comment
The parsing is correct for the targeted use cases. Findings 1-2 are pre-existing infrastructure gaps exposed by this PR — they should be addressed before or shortly after merge for end-to-end functionality. Finding 3 is a forward-compatibility improvement. Finding 4 strengthens test coverage.
Reviewed by glm-5.1 via Qwen Code /review
Problem
OceanBase
CREATE TABLEwithSUBPARTITION TEMPLATEfails with:Example SQL (from OceanBase V4.2.2 documentation):
Root cause: The MySQL parser (used by OceanBase) only recognized
SUBPARTITION OPTIONSafter the subpartition-by clause. When encounteringSUBPARTITION TEMPLATE, it tried to matchOPTIONSand failed.Fix
Add
SUBPARTITION TEMPLATEparsing toMySqlCreateTableParser. When the parser seesTEMPLATEafterSUBPARTITION, it parses the template subpartition list (each containingSUBPARTITION name VALUES ...), storing them in the existingSQLSubPartitionBy.subPartitionTemplatefield.The
SUBPARTITION OPTIONSpath remains unchanged for backward compatibility.Tests Added
4 test cases in
Issue6160.java:test_subpartition_template_list_range— exact SQL from issue (LIST + RANGE template)test_subpartition_template_range_hash— RANGE + HASH templatetest_subpartition_template_with_values_in— RANGE + LIST template with VALUES INtest_subpartition_options_still_works— regression: SUBPARTITIONS count still worksAll 6 existing OceanBase tests pass.
Fixes #6160