From ee05f3d119eff1bb43f11c7ca02af92bbeeb6ea1 Mon Sep 17 00:00:00 2001 From: Yukang-Lian Date: Tue, 13 Aug 2024 16:23:02 +0800 Subject: [PATCH 1/6] 1 --- .../insert/BatchInsertIntoTableCommand.java | 28 ++++++++++++++----- .../trees/plans/physical/PhysicalSink.java | 4 +++ .../org/apache/doris/qe/StmtExecutor.java | 5 ++++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java index c0ab37e5d08fab..39944c2e08855c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java @@ -30,6 +30,7 @@ import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.glue.LogicalPlanAdapter; import org.apache.doris.nereids.trees.TreeNode; +import org.apache.doris.nereids.trees.expressions.NamedExpression; import org.apache.doris.nereids.trees.plans.Explainable; import org.apache.doris.nereids.trees.plans.Plan; import org.apache.doris.nereids.trees.plans.PlanType; @@ -39,6 +40,7 @@ import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; import org.apache.doris.nereids.trees.plans.physical.PhysicalOlapTableSink; import org.apache.doris.nereids.trees.plans.physical.PhysicalOneRowRelation; +import org.apache.doris.nereids.trees.plans.physical.PhysicalSink; import org.apache.doris.nereids.trees.plans.physical.PhysicalUnion; import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; import org.apache.doris.qe.ConnectContext; @@ -51,6 +53,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -143,19 +146,30 @@ public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { Optional union = planner.getPhysicalPlan() .collect(PhysicalUnion.class::isInstance).stream().findAny(); if (union.isPresent()) { - InsertUtils.executeBatchInsertTransaction(ctx, targetTable.getQualifiedDbName(), - targetTable.getName(), targetSchema, union.get().getConstantExprsList()); + InsertUtils.executeBatchInsertTransaction(ctx, targetTable.getQualifiedDbName(), targetTable.getName(), + targetSchema, union.get().getConstantExprsList()); return; } + Optional olapTableSink = planner.getPhysicalPlan() + .collect(PhysicalOlapTableSink.class::isInstance).stream().findAny(); Optional oneRowRelation = planner.getPhysicalPlan() .collect(PhysicalOneRowRelation.class::isInstance).stream().findAny(); - if (oneRowRelation.isPresent()) { - InsertUtils.executeBatchInsertTransaction(ctx, targetTable.getQualifiedDbName(), - targetTable.getName(), targetSchema, ImmutableList.of(oneRowRelation.get().getProjects())); - return; + List outputExprs = ((PhysicalSink) olapTableSink.get()).getOutputExprs(); + List oneRowRelationProjects = oneRowRelation.get().getProjects(); + List rightOutput = new ArrayList(); + for (NamedExpression expr : outputExprs) { + for (NamedExpression project : oneRowRelationProjects) { + if (expr.getExprId().equals(project.getExprId())) { + rightOutput.add(project); + break; + } + } } + InsertUtils.executeBatchInsertTransaction(ctx, targetTable.getQualifiedDbName(), + targetTable.getName(), targetSchema, + ImmutableList.of(rightOutput)); + return; // TODO: update error msg - throw new AnalysisException("could not run this sql"); } finally { targetTableIf.readUnlock(); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalSink.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalSink.java index ed1d6a3a3a7abe..b6b8e5294fbbd6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalSink.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalSink.java @@ -54,4 +54,8 @@ public List computeOutput() { .map(NamedExpression::toSlot) .collect(ImmutableList.toImmutableList()); } + + public List getOutputExprs() { + return outputExprs; + } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java b/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java index e0ae5763abbc43..b6acf6f1126daf 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java @@ -2187,6 +2187,11 @@ private int executeForTxn(InsertStmt insertStmt) if (!(queryStmt instanceof SelectStmt)) { throw new TException("queryStmt is not SelectStmt, insert command error"); } + if (((NativeInsertStmt) insertStmt).getTargetColumnNames() != null) { + throw new TException( + "The legacy planner does not support specifying column names when using ·insert into values`." + + " If you want to specify column names, please `set enable_nereids_planner=true`."); + } TransactionEntry txnEntry = context.getTxnEntry(); SelectStmt selectStmt = (SelectStmt) queryStmt; int effectRows = 0; From fcd02915b3a51574e68f2cc51e9656f59aeb2028 Mon Sep 17 00:00:00 2001 From: Yukang-Lian Date: Tue, 13 Aug 2024 17:47:12 +0800 Subject: [PATCH 2/6] 2 --- .../txn_insert_with_specify_columns.out | 9 +++ .../txn_insert_with_specify_columns.groovy | 69 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 regression-test/data/insert_p0/txn_insert_with_specify_columns.out create mode 100644 regression-test/suites/insert_p0/txn_insert_with_specify_columns.groovy diff --git a/regression-test/data/insert_p0/txn_insert_with_specify_columns.out b/regression-test/data/insert_p0/txn_insert_with_specify_columns.out new file mode 100644 index 00000000000000..a8986772a65408 --- /dev/null +++ b/regression-test/data/insert_p0/txn_insert_with_specify_columns.out @@ -0,0 +1,9 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !select -- +10 20 30 +11 \N 31 +12 22 1 +13 23 33 +14 \N 1 +15 25 35 + diff --git a/regression-test/suites/insert_p0/txn_insert_with_specify_columns.groovy b/regression-test/suites/insert_p0/txn_insert_with_specify_columns.groovy new file mode 100644 index 00000000000000..e5b7403874a70f --- /dev/null +++ b/regression-test/suites/insert_p0/txn_insert_with_specify_columns.groovy @@ -0,0 +1,69 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("txn_insert_with_specify_columns", "p0") { + def table = "txn_insert_with_specify_columns" + + sql """ DROP TABLE IF EXISTS ${table}""" + sql """ + CREATE TABLE ${table}( + c1 INT NULL, + c2 INT NULL, + c3 INT NULL default 1 + ) ENGINE=OLAP + UNIQUE KEY(c1) + DISTRIBUTED BY HASH(c1) BUCKETS 3 + PROPERTIES ( + "replication_num" = "1" + ); + """ + sql """begin""" + sql """insert into ${table} (c1, c3, c2) values(10, 30, 20);""" + sql """insert into ${table} (c3, c1) values(31, 11);""" + sql """insert into ${table} (c2, c1) values(22, 12);""" + sql """insert into ${table} (c2, c3, c1) values(23, 33, 13);""" + sql """insert into ${table} (c1) values(14);""" + sql """insert into ${table} (c3, c2, c1) values(35, 25, 15);""" + sql """commit""" + + try { + sql """set enable_nereids_planner=false""" + sql """ DROP TABLE IF EXISTS ${table}""" + sql """ + CREATE TABLE ${table}( + c1 INT NULL, + c2 INT NULL, + c3 INT NULL default 1 + ) ENGINE=OLAP + UNIQUE KEY(c1) + DISTRIBUTED BY HASH(c1) BUCKETS 3 + PROPERTIES ( + "replication_num" = "1" + ); + """ + sql """begin""" + sql """insert into ${table} (c1, c3, c2) values(10, 30, 20);""" + logger.info(failed) + assertFalse(true); + } catch (Exception e) { + logger.info(e.getMessage()) + assertTrue(e.getMessage().contains("The legacy planner does not support specifying column names")) + } finally { + sql "commit" + sql """ DROP TABLE IF EXISTS ${table}""" + } +} From fce198e812bdeb8c33383e7bfc80ad48b3d64ded Mon Sep 17 00:00:00 2001 From: Yukang-Lian Date: Tue, 13 Aug 2024 21:01:34 +0800 Subject: [PATCH 3/6] 3 --- .../insert/BatchInsertIntoTableCommand.java | 64 ++++++++++++++----- 1 file changed, 47 insertions(+), 17 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java index 39944c2e08855c..99a815d8005bc9 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java @@ -30,6 +30,7 @@ import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.glue.LogicalPlanAdapter; import org.apache.doris.nereids.trees.TreeNode; +import org.apache.doris.nereids.trees.expressions.ExprId; import org.apache.doris.nereids.trees.expressions.NamedExpression; import org.apache.doris.nereids.trees.plans.Explainable; import org.apache.doris.nereids.trees.plans.Plan; @@ -40,7 +41,6 @@ import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; import org.apache.doris.nereids.trees.plans.physical.PhysicalOlapTableSink; import org.apache.doris.nereids.trees.plans.physical.PhysicalOneRowRelation; -import org.apache.doris.nereids.trees.plans.physical.PhysicalSink; import org.apache.doris.nereids.trees.plans.physical.PhysicalUnion; import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; import org.apache.doris.qe.ConnectContext; @@ -54,7 +54,9 @@ import org.apache.logging.log4j.Logger; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; @@ -147,27 +149,18 @@ public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { .collect(PhysicalUnion.class::isInstance).stream().findAny(); if (union.isPresent()) { InsertUtils.executeBatchInsertTransaction(ctx, targetTable.getQualifiedDbName(), targetTable.getName(), - targetSchema, union.get().getConstantExprsList()); + targetSchema, reorderUnionData(sink.getOutputExprs(), union.get().getOutputs(), + union.get().getConstantExprsList())); return; } - Optional olapTableSink = planner.getPhysicalPlan() - .collect(PhysicalOlapTableSink.class::isInstance).stream().findAny(); Optional oneRowRelation = planner.getPhysicalPlan() .collect(PhysicalOneRowRelation.class::isInstance).stream().findAny(); - List outputExprs = ((PhysicalSink) olapTableSink.get()).getOutputExprs(); - List oneRowRelationProjects = oneRowRelation.get().getProjects(); - List rightOutput = new ArrayList(); - for (NamedExpression expr : outputExprs) { - for (NamedExpression project : oneRowRelationProjects) { - if (expr.getExprId().equals(project.getExprId())) { - rightOutput.add(project); - break; - } - } + if (oneRowRelation.isPresent()) { + InsertUtils.executeBatchInsertTransaction(ctx, targetTable.getQualifiedDbName(), + targetTable.getName(), targetSchema, + ImmutableList.of( + reorderOneRowData(sink.getOutputExprs(), oneRowRelation.get().getProjects()))); } - InsertUtils.executeBatchInsertTransaction(ctx, targetTable.getQualifiedDbName(), - targetTable.getName(), targetSchema, - ImmutableList.of(rightOutput)); return; // TODO: update error msg } finally { @@ -175,6 +168,43 @@ public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { } } + // If table schema is c1, c2, c3, we do insert into table (c3, c2, c1) values(v3, v2, v1). + // The oneRowExprts are [v3#c1, v2#c2, v1#c3], which is wrong sequence. The sinkExprs are + // [v1#c3, v2#c2, v3#c1]. However, sinkExprs are SlotRefrence rather than Alias. We need to + // extract right sequence alias from oneRowExprs. + private List reorderOneRowData(List sinkExprs, + List oneRowExprs) { + List sequenceData = new ArrayList<>(); + for (NamedExpression expr : sinkExprs) { + for (NamedExpression project : oneRowExprs) { + if (expr.getExprId().equals(project.getExprId())) { + sequenceData.add(project); + break; + } + } + } + return sequenceData; + } + + private List> reorderUnionData(List sinkExprs, + List unionOutputs, List> unionExprs) { + Map indexMap = new HashMap<>(); + for (int i = 0; i < unionOutputs.size(); i++) { + indexMap.put(unionOutputs.get(i).getExprId(), i); + } + + List> reorderedExprs = new ArrayList<>(); + for (List exprList : unionExprs) { + List reorderedList = new ArrayList<>(); + for (NamedExpression expr : sinkExprs) { + int index = indexMap.get(expr.getExprId()); + reorderedList.add(exprList.get(index)); + } + reorderedExprs.add(reorderedList); + } + return reorderedExprs; + } + @Override public StmtType stmtType() { return StmtType.INSERT; From 902f312e42b0b85ee2a982eec6ce6853e9034a5c Mon Sep 17 00:00:00 2001 From: Yukang-Lian Date: Tue, 13 Aug 2024 21:25:10 +0800 Subject: [PATCH 4/6] 4 --- .../txn_insert_with_specify_columns.out | 16 +++++- .../txn_insert_with_specify_columns.groovy | 55 ++++++++++++++++++- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/regression-test/data/insert_p0/txn_insert_with_specify_columns.out b/regression-test/data/insert_p0/txn_insert_with_specify_columns.out index a8986772a65408..206d6b429b0675 100644 --- a/regression-test/data/insert_p0/txn_insert_with_specify_columns.out +++ b/regression-test/data/insert_p0/txn_insert_with_specify_columns.out @@ -1,9 +1,21 @@ -- This file is automatically generated. You should know what you did if you want to edit this --- !select -- -10 20 30 +-- !select_unique -- +1 2 3 0 +11 \N 31 0 +12 22 1 0 +13 23 33 0 +14 \N 1 0 +15 25 35 1 +111 222 333 0 +1111 2222 3333 0 + +-- !select_unique -- +1 2 3 11 \N 31 12 22 1 13 23 33 14 \N 1 15 25 35 +111 222 333 +1111 2222 3333 diff --git a/regression-test/suites/insert_p0/txn_insert_with_specify_columns.groovy b/regression-test/suites/insert_p0/txn_insert_with_specify_columns.groovy index e5b7403874a70f..495b84aed788b8 100644 --- a/regression-test/suites/insert_p0/txn_insert_with_specify_columns.groovy +++ b/regression-test/suites/insert_p0/txn_insert_with_specify_columns.groovy @@ -32,13 +32,39 @@ suite("txn_insert_with_specify_columns", "p0") { ); """ sql """begin""" - sql """insert into ${table} (c1, c3, c2) values(10, 30, 20);""" + sql """insert into ${table} (c1, c3, c2) values(1, 3, 2),(111,333,222),(1111,3333,2222);""" sql """insert into ${table} (c3, c1) values(31, 11);""" sql """insert into ${table} (c2, c1) values(22, 12);""" sql """insert into ${table} (c2, c3, c1) values(23, 33, 13);""" sql """insert into ${table} (c1) values(14);""" sql """insert into ${table} (c3, c2, c1) values(35, 25, 15);""" + sql """insert into ${table} (__DORIS_DELETE_SIGN__, c3, c2, c1) values(1, 35, 25, 15);""" sql """commit""" + sql """set show_hidden_columns=true""" + qt_select_unique """select c1,c2,c3,__DORIS_DELETE_SIGN__ from ${table} order by c1,c2,c3""" + + sql """ DROP TABLE IF EXISTS ${table}""" + sql """ + CREATE TABLE ${table}( + c1 INT NULL, + c2 INT NULL, + c3 INT NULL default 1 + ) ENGINE=OLAP + DUPLICATE KEY(c1) + DISTRIBUTED BY HASH(c1) BUCKETS 3 + PROPERTIES ( + "replication_num" = "1" + ); + """ + sql """begin""" + sql """insert into ${table} (c1, c3, c2) values(1, 3, 2),(111,333,222),(1111,3333,2222);""" + sql """insert into ${table} (c3, c1) values(31, 11);""" + sql """insert into ${table} (c2, c1) values(22, 12);""" + sql """insert into ${table} (c2, c3, c1) values(23, 33, 13);""" + sql """insert into ${table} (c1) values(14);""" + sql """insert into ${table} (c3, c2, c1) values(35, 25, 15);""" + sql """commit""" + qt_select_unique """select c1,c2,c3 from ${table} order by c1,c2,c3""" try { sql """set enable_nereids_planner=false""" @@ -66,4 +92,31 @@ suite("txn_insert_with_specify_columns", "p0") { sql "commit" sql """ DROP TABLE IF EXISTS ${table}""" } + + try { + sql """set enable_nereids_planner=false""" + sql """ DROP TABLE IF EXISTS ${table}""" + sql """ + CREATE TABLE ${table}( + c1 INT NULL, + c2 INT NULL, + c3 INT NULL default 1 + ) ENGINE=OLAP + DUPLICATE KEY(c1) + DISTRIBUTED BY HASH(c1) BUCKETS 3 + PROPERTIES ( + "replication_num" = "1" + ); + """ + sql """begin""" + sql """insert into ${table} (c1, c3, c2) values(10, 30, 20);""" + logger.info(failed) + assertFalse(true); + } catch (Exception e) { + logger.info(e.getMessage()) + assertTrue(e.getMessage().contains("The legacy planner does not support specifying column names")) + } finally { + sql "commit" + sql """ DROP TABLE IF EXISTS ${table}""" + } } From 38bb93f3bbab6fac7ebace6b3a10421ecce95844 Mon Sep 17 00:00:00 2001 From: Yukang-Lian Date: Tue, 13 Aug 2024 21:38:28 +0800 Subject: [PATCH 5/6] 5 --- .../data_model_p0/unique/test_unique_table_new_sequence.groovy | 2 ++ .../data_model_p0/unique/test_unique_table_sequence.groovy | 3 +++ 2 files changed, 5 insertions(+) diff --git a/regression-test/suites/data_model_p0/unique/test_unique_table_new_sequence.groovy b/regression-test/suites/data_model_p0/unique/test_unique_table_new_sequence.groovy index be8b4291e3a894..09844709f5cf71 100644 --- a/regression-test/suites/data_model_p0/unique/test_unique_table_new_sequence.groovy +++ b/regression-test/suites/data_model_p0/unique/test_unique_table_new_sequence.groovy @@ -171,6 +171,8 @@ suite("test_unique_table_new_sequence") { qt_1 "select * from ${tableName} order by k1;" + sql """set enable_nereids_dml = true; """ + sql """set enable_nereids_planner=true""" sql "begin;" sql "insert into ${tableName} (k1, v1, v2, v3, `OR`) values (2,20,20,20,20);" sql "commit;" diff --git a/regression-test/suites/data_model_p0/unique/test_unique_table_sequence.groovy b/regression-test/suites/data_model_p0/unique/test_unique_table_sequence.groovy index db4df6c9e34364..9648d257f58486 100644 --- a/regression-test/suites/data_model_p0/unique/test_unique_table_sequence.groovy +++ b/regression-test/suites/data_model_p0/unique/test_unique_table_sequence.groovy @@ -173,6 +173,9 @@ suite("test_unique_table_sequence") { } sql "commit;" + sql """set enable_nereids_dml = true; """ + sql """set enable_nereids_planner=true""" + sql "begin;" sql "insert into ${tableName} (k1, v1, v2, v3, `or`, __doris_sequence_col__) values (1,1,1,1,1,1),(2,2,2,2,2,2),(3,3,3,3,3,3);" sql "commit;" From fad1a2c8add922ed651dfa4535c37749ed5a4fa7 Mon Sep 17 00:00:00 2001 From: Yukang-Lian Date: Wed, 14 Aug 2024 11:17:13 +0800 Subject: [PATCH 6/6] 6 --- .../plans/commands/insert/BatchInsertIntoTableCommand.java | 3 ++- .../suites/insert_p0/insert_group_commit_into.groovy | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java index 99a815d8005bc9..a1b834fe904cfb 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/BatchInsertIntoTableCommand.java @@ -160,9 +160,10 @@ targetSchema, reorderUnionData(sink.getOutputExprs(), union.get().getOutputs(), targetTable.getName(), targetSchema, ImmutableList.of( reorderOneRowData(sink.getOutputExprs(), oneRowRelation.get().getProjects()))); + return; } - return; // TODO: update error msg + throw new AnalysisException("could not run this sql"); } finally { targetTableIf.readUnlock(); } diff --git a/regression-test/suites/insert_p0/insert_group_commit_into.groovy b/regression-test/suites/insert_p0/insert_group_commit_into.groovy index 9714fec4210af1..79e62505ad40e4 100644 --- a/regression-test/suites/insert_p0/insert_group_commit_into.groovy +++ b/regression-test/suites/insert_p0/insert_group_commit_into.groovy @@ -242,6 +242,9 @@ suite("insert_group_commit_into") { assertEquals(23, rowCount[0][0]) // txn insert + sql """ set enable_nereids_dml = true; """ + sql """ set enable_nereids_planner=true; """ + sql """ set enable_fallback_to_original_planner=false; """ def stmt = prepareStatement """ begin """ stmt.executeUpdate() txn_insert """ insert into ${table}(id, name, score) values(20, 'i', 101); """, 1