-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[feature](lineage) Support lineage when ctas, insert into and insert overwrite #60122
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.
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
base: master
Are you sure you want to change the base?
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
1 similar comment
|
run buildall |
TPC-H: Total hot run time: 31117 ms |
TPC-DS: Total hot run time: 173439 ms |
ClickBench: Total hot run time: 26.8 s |
FE UT Coverage ReportIncrement line coverage |
81e42fc to
022d0b6
Compare
|
run buildall |
TPC-H: Total hot run time: 31075 ms |
TPC-DS: Total hot run time: 172742 ms |
| if (!LineageUtils.isSameParsedCommand(executor, currentHandleClass)) { | ||
| return; | ||
| } | ||
| if (!isLineagePluginConfigured()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should check this one at first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have fixed
| && FeConstants.INTERNAL_DB_NAME.equalsIgnoreCase(dbName); | ||
| } | ||
|
|
||
| private static Map<String, Map<String, String>> collectExternalCatalogProperties(LineageInfo lineageInfo) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why need catalog properties?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because some lineage plugin need external catalog property such as uri and so on
| package org.apache.doris.nereids.lineage; | ||
|
|
||
| /** | ||
| * Lineage event wrapping lineage info for plugins. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why need this wrapper?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lineage info is enough, remove the lineage event
| lineagePlugins = Collections.emptyList(); | ||
| } | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("update lineage plugins. num: {}", lineagePlugins.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should print names here
| for (Plugin plugin : lineagePlugins) { | ||
| try { | ||
| AbstractLineagePlugin lineagePlugin = (AbstractLineagePlugin) plugin; | ||
| if (!lineagePlugin.eventFilter()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eventFilter do not need Event as parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this judge by the config parm in fe.conf, which can be get by Config.xxx,so not need parameter now
| LineageInfo lineageInfo = lineageEvent.getLineageInfo(); | ||
| if (lineageInfo == null) { | ||
| LOG.warn("lineage info is null for event {}, skip", getQueryId(lineageEvent)); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move it as the first step
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have fixed
| return; | ||
| } | ||
| CascadesContext cascadesContext = getCascadesContext(); | ||
| getStatementContext().getPlannerHooks().forEach(hook -> hook.beforeAnalyze(this)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if insert values does not generate lineage event, why add hoot here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
|
run buildall |
TPC-H: Total hot run time: 32857 ms |
ClickBench: Total hot run time: 28.22 s |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
TPC-H: Total hot run time: 32296 ms |
ClickBench: Total hot run time: 28.34 s |
|
run buildall |
TPC-H: Total hot run time: 32926 ms |
ClickBench: Total hot run time: 28.23 s |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
1 similar comment
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
TPC-H: Total hot run time: 31639 ms |
ClickBench: Total hot run time: 28.71 s |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
TPC-H: Total hot run time: 31798 ms |
ClickBench: Total hot run time: 28.18 s |
What problem does this PR solve?
Code Flow
Design
Example (JOIN)
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)