-
Notifications
You must be signed in to change notification settings - Fork 434
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
[GLUTEN-7145][CH] Decouple SerializedPlanParser
from other parser modules
#7250
Conversation
Run Gluten Clickhouse CI |
9c0563f
to
efa5e9a
Compare
Run Gluten Clickhouse CI |
efa5e9a
to
a8fae59
Compare
Run Gluten Clickhouse CI |
a8fae59
to
7d82d57
Compare
Run Gluten Clickhouse CI |
7d82d57
to
78a3468
Compare
Run Gluten Clickhouse CI |
78a3468
to
e02a350
Compare
Run Gluten Clickhouse CI |
e02a350
to
86ed0fb
Compare
Run Gluten Clickhouse CI |
1 similar comment
Run Gluten Clickhouse CI |
f067253
to
00edd36
Compare
Run Gluten Clickhouse CI |
00edd36
to
6e7e1ef
Compare
Run Gluten Clickhouse CI |
6e7e1ef
to
3f02aa5
Compare
Run Gluten Clickhouse CI |
3f02aa5
to
c0a3f56
Compare
Run Gluten Clickhouse CI |
c0a3f56
to
207935d
Compare
Run Gluten Clickhouse CI |
207935d
to
b0ed306
Compare
Run Gluten Clickhouse CI |
ExpressionParser
SerializedPlanParser
from other parser modules
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.
LGTM
@lgbo-ustc please hold this PR |
? |
b0ed306
to
1ba30da
Compare
Run Gluten Clickhouse CI |
1ba30da
to
067672f
Compare
Run Gluten Clickhouse CI |
067672f
to
14badae
Compare
Run Gluten Clickhouse CI |
14badae
to
3cf91e5
Compare
Run Gluten Clickhouse CI |
Run Gluten Clickhouse CI |
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.
LGTM
===== Performance report for TPCDS SF2000 with Velox backend, for reference only ====
|
What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
Fixes: #7145
Decouple
SerializedPlanParser
from all other modules. IntroduceParserContext
andExpressionParser
in this PR.ParserContext
is the minimal core that need be shared among parser modules.ExpressionParser
is responsilbe for parser substrait expession intoCH
actions dag. It unifies this tranformation among all parser modules. Base on this, some optimization will be applied at later.All these should make the code more flexible.
How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
unit tests
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)