morrySnow commented on code in PR #32156:
URL: https://github.com/apache/doris/pull/32156#discussion_r1528171145


##########
docs/zh-CN/docs/sql-manual/sql-reference/Data-Manipulation-Statements/Manipulation/INSERT-OVERWRITE.md:
##########
@@ -176,7 +186,56 @@ PROPERTIES (
     INSERT OVERWRITE table test PARTITION(p1,p2) WITH LABEL `label4` (c1, c2) 
SELECT * from test2;
     ```
 
+#### Overwrite Auto Detect Partition
+
+当 INSERT OVERWRITE 命令指定的 PARTITION 子句为 `PARTITION(*)` 时,此次覆写将会自动检测分区数据所在的分区。例如:
+
+```sql
+mysql> create table test(
+    -> k0 int null
+    -> )
+    -> partition by range (k0)
+    -> (
+    -> PARTITION p10 values less than (10),
+    -> PARTITION p100 values less than (100),
+    -> PARTITION pMAX values less than (maxvalue)
+    -> )
+    -> DISTRIBUTED BY HASH(`k0`) BUCKETS 1
+    -> properties("replication_num" = "1");
+Query OK, 0 rows affected (0.11 sec)
+
+mysql> insert into test values (1), (2), (15), (100), (200);
+Query OK, 5 rows affected (0.29 sec)
+
+mysql> select * from test order by k0;
++------+
+| k0   |
++------+
+|    1 |
+|    2 |
+|   15 |
+|  100 |
+|  200 |
++------+
+5 rows in set (0.23 sec)
+
+mysql> insert overwrite table test partition(*) values (3), (1234);

Review Comment:
   如果数据对应的分区不存在,会发生什么?



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/analyzer/UnboundTableSinkCreator.java:
##########
@@ -71,4 +72,21 @@ public static LogicalSink<? extends Plan> 
createUnboundTableSink(List<String> na
         }
         throw new RuntimeException("Load data to " + 
curCatalog.getClass().getSimpleName() + " is not supported.");
     }
+
+    /**
+     * create unbound sink for DML plan with auto detect overwrite partition 
enable
+     */
+    public static LogicalSink<? extends Plan> 
createUnboundTableSink(List<String> nameParts,
+            List<String> colNames, List<String> hints,
+            boolean isPartialUpdate, DMLCommandType dmlCommandType, 
LogicalPlan plan) {
+        String catalogName = 
RelationUtil.getQualifierName(ConnectContext.get(), nameParts).get(0);
+        CatalogIf<?> curCatalog = 
Env.getCurrentEnv().getCatalogMgr().getCatalog(catalogName);
+        if (curCatalog instanceof InternalCatalog) {
+            return new UnboundTableSink<>(nameParts, colNames, hints, false, 
Lists.newArrayList(), true,

Review Comment:
   do not use `Lists.newArrayList()`, use `ImmutableList.of()`



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/analyzer/UnboundTableSinkCreator.java:
##########
@@ -28,6 +28,7 @@
 import org.apache.doris.nereids.trees.plans.logical.LogicalSink;
 import org.apache.doris.nereids.util.RelationUtil;
 import org.apache.doris.qe.ConnectContext;
+import org.checkerframework.com.google.common.collect.Lists;

Review Comment:
   use `com.google.common.collect.Lists`



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -1106,6 +1106,12 @@ public Set<String> getPartitionNames() {
         return Sets.newHashSet(nameToPartition.keySet());
     }
 
+    public List<String> getPartitionNamesByIds(List<Long> partitionIds) {
+        return partitionIds.stream().map(id -> {

Review Comment:
   do we call it under thread safe scene?



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -527,17 +527,33 @@ public LogicalPlan visitInsertTable(InsertTableContext 
ctx) {
         Optional<String> labelName = ctx.labelName == null ? Optional.empty() 
: Optional.of(ctx.labelName.getText());
         List<String> colNames = ctx.cols == null ? ImmutableList.of() : 
visitIdentifierList(ctx.cols);
         // TODO visit partitionSpecCtx
-        Pair<Boolean, List<String>> partitionSpec = 
visitPartitionSpec(ctx.partitionSpec());
         LogicalPlan plan = visitQuery(ctx.query());
-        LogicalSink<?> sink = UnboundTableSinkCreator.createUnboundTableSink(
-                tableName.build(),
-                colNames,
-                ImmutableList.of(),
-                partitionSpec.first,
-                partitionSpec.second,
-                
ConnectContext.get().getSessionVariable().isEnableUniqueKeyPartialUpdate(),
-                DMLCommandType.INSERT,
-                plan);
+        // partitionSpec may be NULL. means auto detect partition. only 
available when
+        // IOT
+        Pair<Boolean, List<String>> partitionSpec = 
visitPartitionSpec(ctx.partitionSpec());
+        LogicalSink<?> sink;
+        if (partitionSpec.second == null) { // auto detect partition
+            if (!isOverwrite) {
+                throw new ParseException("Only support wildcard in overwrite 
partition", ctx);
+            }

Review Comment:
   this exception should throw in `createUnboundTableSink`



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/PartitionNames.java:
##########
@@ -48,36 +48,36 @@ public class PartitionNames implements ParseNode, Writable {
     // true if these partitions are temp partitions
     @SerializedName(value = "isTemp")
     private final boolean isTemp;
-    private final boolean allPartitions;
+    private final boolean autoReplace;

Review Comment:
   i think the better name is isStar, because we use it to represent different 
syntax in different statement. 



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -527,17 +527,33 @@ public LogicalPlan visitInsertTable(InsertTableContext 
ctx) {
         Optional<String> labelName = ctx.labelName == null ? Optional.empty() 
: Optional.of(ctx.labelName.getText());
         List<String> colNames = ctx.cols == null ? ImmutableList.of() : 
visitIdentifierList(ctx.cols);
         // TODO visit partitionSpecCtx
-        Pair<Boolean, List<String>> partitionSpec = 
visitPartitionSpec(ctx.partitionSpec());
         LogicalPlan plan = visitQuery(ctx.query());
-        LogicalSink<?> sink = UnboundTableSinkCreator.createUnboundTableSink(
-                tableName.build(),
-                colNames,
-                ImmutableList.of(),
-                partitionSpec.first,
-                partitionSpec.second,
-                
ConnectContext.get().getSessionVariable().isEnableUniqueKeyPartialUpdate(),
-                DMLCommandType.INSERT,
-                plan);
+        // partitionSpec may be NULL. means auto detect partition. only 
available when
+        // IOT
+        Pair<Boolean, List<String>> partitionSpec = 
visitPartitionSpec(ctx.partitionSpec());
+        LogicalSink<?> sink;
+        if (partitionSpec.second == null) { // auto detect partition

Review Comment:
   if statement should move into createUnboundTableSink



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/analyzer/UnboundTableSink.java:
##########
@@ -50,6 +52,7 @@ public class UnboundTableSink<CHILD_TYPE extends Plan> 
extends UnboundLogicalSin
     private final List<String> partitions;
     private final boolean isPartialUpdate;
     private final DMLCommandType dmlCommandType;
+    private boolean autoDetectPartition = false;

Review Comment:
   make it final



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -1106,6 +1106,12 @@ public Set<String> getPartitionNames() {
         return Sets.newHashSet(nameToPartition.keySet());
     }
 
+    public List<String> getPartitionNamesByIds(List<Long> partitionIds) {
+        return partitionIds.stream().map(id -> {
+            return idToPartition.get(id).getName();
+        }).collect(Collectors.toList());

Review Comment:
   avoid use stream api for better performance



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/analyzer/UnboundTableSinkCreator.java:
##########
@@ -71,4 +72,21 @@ public static LogicalSink<? extends Plan> 
createUnboundTableSink(List<String> na
         }
         throw new RuntimeException("Load data to " + 
curCatalog.getClass().getSimpleName() + " is not supported.");
     }
+
+    /**
+     * create unbound sink for DML plan with auto detect overwrite partition 
enable
+     */
+    public static LogicalSink<? extends Plan> 
createUnboundTableSink(List<String> nameParts,
+            List<String> colNames, List<String> hints,
+            boolean isPartialUpdate, DMLCommandType dmlCommandType, 
LogicalPlan plan) {
+        String catalogName = 
RelationUtil.getQualifierName(ConnectContext.get(), nameParts).get(0);
+        CatalogIf<?> curCatalog = 
Env.getCurrentEnv().getCatalogMgr().getCatalog(catalogName);
+        if (curCatalog instanceof InternalCatalog) {
+            return new UnboundTableSink<>(nameParts, colNames, hints, false, 
Lists.newArrayList(), true,
+                    isPartialUpdate, dmlCommandType, Optional.empty(),
+                    Optional.empty(), plan);
+        }
+        throw new RuntimeException(

Review Comment:
   throw AnalyzeException



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -567,7 +583,9 @@ public Pair<Boolean, List<String>> 
visitPartitionSpec(PartitionSpecContext ctx)
         boolean temporaryPartition = false;
         if (ctx != null) {
             temporaryPartition = ctx.TEMPORARY() != null;
-            if (ctx.partition != null) {
+            if (ctx.ASTERISK() != null) {

Review Comment:
   nit: it is better to give ASTERISK a alias name



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to