Adding support for renaming tables and partitioned tables #5574
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5574 +/- ##
============================================
+ Coverage 82.66% 82.89% +0.23%
- Complexity 11590 11702 +112
============================================
Files 1382 1390 +8
Lines 44914 45200 +286
Branches 5580 5601 +21
============================================
+ Hits 37128 37469 +341
+ Misses 5174 5130 -44
+ Partials 2612 2601 -11
Continue to review full report at Codecov.
|
| @@ -56,6 +57,23 @@ public AlterTableAnalyzedStatement analyze(AlterTable node, Row parameters, Sess | |||
| return new AlterTableAnalyzedStatement(docTableInfo, partitionName, tableParameter, table.excludePartitions()); | |||
| } | |||
|
|
|||
| AlterTableRenameAnalyzedStatement analyzeRename(AlterTableRename node, String defaultSchema, | |||
There was a problem hiding this comment.
isn't the defaultSchema part of the sessionContext? I'd either require only the sessionContext or replace the sessionContext with user so that both dependencies are explicit, but not a mixture.
|
|
||
| List<String> newIdentParts = node.newName().getParts(); | ||
| if (newIdentParts.size() > 1) { | ||
| throw new IllegalArgumentException("Target table name must not include a schema"); |
There was a problem hiding this comment.
Is there a (technical) constraint that we don't allow this? If so - maybe add a short code comment to mention it
There was a problem hiding this comment.
It also seems as if there is no test for this?
| public class TransportRenameTableAction extends TransportMasterNodeAction<RenameTableRequest, RenameTableResponse> { | ||
|
|
||
| private static final String ACTION_NAME = "crate/table/rename"; | ||
| private static final IndicesOptions INDICES_OPTIONS = IndicesOptions.fromOptions(false, false, false, false); |
There was a problem hiding this comment.
Please rename so that it includes the characteristics of the IndicesOptions in the name. STRICT_INDICES_OPTIONS or something like that.
| TableIdent tableIdent = TableIdent.of(node.table(), defaultSchema); | ||
| DocTableInfo tableInfo = schemas.getTableInfo(tableIdent, sessionContext.user()); | ||
| if (!node.table().partitionProperties().isEmpty()) { | ||
| throw new UnsupportedOperationException("Renaming a single partition is not supported"); |
There was a problem hiding this comment.
Is there a analyzer-unit-test for this?
The related table will be closed upfront renaming (if not already closed), same for a partitioned table (close all non-closed partitions) and re-opened afterwards (only re-open if table or partition was open before).