morrySnow commented on code in PR #31666:
URL: https://github.com/apache/doris/pull/31666#discussion_r1510908499
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -531,7 +531,8 @@ public LogicalPlan visitInsertTable(InsertTableContext ctx)
{
if (isOverwrite) {
command = new InsertOverwriteTableCommand(sink, labelName);
} else {
- if (ConnectContext.get() != null &&
ConnectContext.get().isTxnModel()) {
+ if (ConnectContext.get() != null &&
ConnectContext.get().isTxnModel()
+ && sink.child() instanceof LogicalInlineTable) {
Review Comment:
In legacy planner, we support `insert into t select 1`. So if we only
support `LogicalInlineTable` here means we have diff with legacy planner
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTableCommand.java:
##########
@@ -146,13 +146,14 @@ private void runInternal(ConnectContext ctx, StmtExecutor
executor) throws Excep
Preconditions.checkArgument(plan.isPresent(), "insert into command
must contain target table");
PhysicalSink physicalSink = plan.get();
DataSink sink = planner.getFragments().get(0).getSink();
- String label = this.labelName.orElse(String.format("label_%x_%x",
ctx.queryId().hi, ctx.queryId().lo));
if (physicalSink instanceof PhysicalOlapTableSink) {
if (GroupCommitInserter.groupCommit(ctx, sink, physicalSink)) {
return;
}
OlapTable olapTable = (OlapTable) targetTableIf;
+ String label = this.labelName.orElse(
+ ctx.isTxnModel() ? null : String.format("label_%x_%x",
ctx.queryId().hi, ctx.queryId().lo));
Review Comment:
could u add some comment to explain why lable should be null when we in txn
model
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertUtils.java:
##########
@@ -162,6 +162,10 @@ private static InternalService.PDataRow
getRowStringValue(List<NamedExpression>
private static void beginBatchInsertTransaction(ConnectContext ctx,
String dbName, String tblName, List<Column> columns) {
TransactionEntry txnEntry = ctx.getTxnEntry();
+ if (txnEntry.isTransactionBegan()) {
+ throw new AnalysisException(
+ "Transaction insert can not insert into values and insert
into select at the same time");
Review Comment:
why `isTransactionBegan` means use insert into values with insert into
select at same time?
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -507,10 +505,12 @@ public void execute(TUniqueId queryId) throws Exception {
LOG.warn("Analyze failed. {}",
context.getQueryIdentifier(), e);
throw ((NereidsException) e).getException();
}
- boolean isInsertIntoCommand = parsedStmt != null &&
parsedStmt instanceof LogicalPlanAdapter
- && ((LogicalPlanAdapter)
parsedStmt).getLogicalPlan() instanceof InsertIntoTableCommand;
- if (e instanceof NereidsException
- &&
!context.getSessionVariable().enableFallbackToOriginalPlanner &&
!isInsertIntoCommand) {
+ boolean isGroupCommit = (parsedStmt != null
+ && parsedStmt instanceof LogicalPlanAdapter
+ && ((LogicalPlanAdapter)
parsedStmt).getLogicalPlan() instanceof InsertIntoTableCommand)
+ && !context.isTxnModel();
Review Comment:
so all insert into without txn model is group commit?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
