Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[opt](profile) Disable profile for insert into values and other commands #47296

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+48 −1
Open

[opt](profile) Disable profile for insert into values and other commands #47296

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cd27a00
DE
zhiqiang-hhhh Jan 21, 2025
06df124
NED
zhiqiang-hhhh Jan 21, 2025
5dfd8ce
F
zhiqiang-hhhh Jan 22, 2025
4df958b
N
zhiqiang-hhhh Jan 22, 2025
01dbb1c
FIX EXTENAL
zhiqiang-hhhh Jan 22, 2025
4078c2f
CR
zhiqiang-hhhh Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,22 @@
import org.apache.doris.nereids.NereidsPlanner;
import org.apache.doris.nereids.PlanProcess;
import org.apache.doris.nereids.StatementContext;
import org.apache.doris.nereids.analyzer.UnboundTableSink;
import org.apache.doris.nereids.exceptions.MustFallbackException;
import org.apache.doris.nereids.exceptions.ParseException;
import org.apache.doris.nereids.glue.LogicalPlanAdapter;
import org.apache.doris.nereids.minidump.MinidumpUtils;
import org.apache.doris.nereids.parser.NereidsParser;
import org.apache.doris.nereids.rules.exploration.mv.InitMaterializationContextHook;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.algebra.InlineTable;
import org.apache.doris.nereids.trees.plans.commands.Command;
import org.apache.doris.nereids.trees.plans.commands.CreatePolicyCommand;
import org.apache.doris.nereids.trees.plans.commands.CreateTableCommand;
import org.apache.doris.nereids.trees.plans.commands.DeleteFromCommand;
import org.apache.doris.nereids.trees.plans.commands.DeleteFromUsingCommand;
import org.apache.doris.nereids.trees.plans.commands.Forward;
import org.apache.doris.nereids.trees.plans.commands.LoadCommand;
import org.apache.doris.nereids.trees.plans.commands.PrepareCommand;
import org.apache.doris.nereids.trees.plans.commands.Redirect;
import org.apache.doris.nereids.trees.plans.commands.UnsupportedCommand;
Expand Down Expand Up @@ -1226,6 +1230,40 @@ private boolean isQuery() {
&& !(((LogicalPlanAdapter) parsedStmt).getLogicalPlan() instanceof Command));
}

private boolean isProfileSafeStmt() {
// fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java:131
// Only generate profile for NereidsPlanner.
if (!(parsedStmt instanceof LogicalPlanAdapter)) {
return false;
}

LogicalPlan plan = ((LogicalPlanAdapter) parsedStmt).getLogicalPlan();

if (plan instanceof InsertIntoTableCommand) {
LogicalPlan logicalPlan = ((InsertIntoTableCommand) plan).getLogicalQuery();
if (logicalPlan instanceof UnboundTableSink) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if UnboundJdbcSink or UnboundHiveSink? is it safe for them to generate profile not like olap table?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed by adding UnboundBaseExternalTableSink

if (logicalPlan.children() == null || logicalPlan.children().isEmpty()) {
return false;
}

for (Plan child : logicalPlan.children()) {
if (child instanceof InlineTable) {
return false;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comment to explain what does InlineTable mean

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

return true;
}

// Generate profile for CreateTableCommand(mainly for create as select).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment is different with code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if ((plan instanceof Command) && !(plan instanceof LoadCommand)
&& !(plan instanceof CreateTableCommand) && !(plan instanceof InsertOverwriteTableCommand)) {
return false;
} else {
return true;
}
}

private void forwardToMaster() throws Exception {
masterOpExecutor = new MasterOpExecutor(originStmt, context, redirectStatus, isQuery());
if (LOG.isDebugEnabled()) {
Expand Down Expand Up @@ -1261,7 +1299,7 @@ private void forwardToMaster() throws Exception {
}

public void updateProfile(boolean isFinished) {
if (!context.getSessionVariable().enableProfile()) {
if (!context.getSessionVariable().enableProfile() || !isProfileSafeStmt()) {
return;
}
// If any error happened in update profile, we should ignore this error
Expand Down
Loading