-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[Improvement](Nereids) Support create routine load command #43930
[Improvement](Nereids) Support create routine load command #43930
Conversation
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
b99ed23
to
4e88e1c
Compare
run buildall |
1 similar comment
run buildall |
dd206f7
to
2d40ee2
Compare
run buildall |
eafb593
to
16b0b4c
Compare
run buildall |
16b0b4c
to
e62b863
Compare
run buildall |
2 similar comments
run buildall |
run buildall |
77eac8c
to
9480305
Compare
run buildall |
} | ||
|
||
private void checkDBTable(ConnectContext ctx) throws AnalysisException { | ||
dbName = labelName.getDbName(); |
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.
Better to create a new LabelNameInfo class and implement analyze or validate method like TableNameInfo. Because LabelNameInfo may be used in other commands, and the validate can be called in multiple place.
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.
done
} | ||
} | ||
} | ||
routineLoadDesc = new RoutineLoadDesc(columnSeparator, lineDelimiter, importColumnsStmt, |
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.
Better to create a new RoutineLoadInfo class and remove the dependence of ImportColumnsStmt, ImportWhereStmt. Just keep Expression as its member variable. And add setRoutineLoadDesc(RoutineLoadInfo info) into RoutineLoadJob to use the new class.(you can do it in next pr)
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.
got it
List<String> nameParts = Lists.newArrayList(); | ||
nameParts.add(dbName); | ||
nameParts.add(tableName); | ||
Plan unboundRelation = new UnboundRelation(StatementScopeIdGenerator.newRelationId(), nameParts); |
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.
bind an Expression and translate to legacy Expr is used very often. Better to put this into a new ExpressionToExprUtil class
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.
the initialize of translation differs a lot, so it is hard to extract one common analyzer and translator. In next pr using it I would extract common codes
This reverts commit 447489c.
06bd4c6
to
5fb7d84
Compare
run buildall |
run buildall |
run feut |
run buildall |
run buildall |
run feut |
1 similar comment
run feut |
run cloud_p0 |
1 similar comment
run cloud_p0 |
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
What problem does this PR solve?
Issue Number: close #42805
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)