-
Notifications
You must be signed in to change notification settings - Fork 590
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
feat(optimizer): turn on logical rewrite for stream #1231
feat(optimizer): turn on logical rewrite for stream #1231
Conversation
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
@@ -64,8 +60,8 @@ | |||
BatchProject { exprs: [Case(($0 = 1:Int32), 1:Int32::Decimal, ($0 = 2:Int32), 2:Int32::Decimal, Normalized(0.0):Decimal)], expr_alias: [ ] } | |||
BatchScan { table: t, columns: [v1] } | |||
stream_plan: | | |||
StreamMaterialize { table_id: 0, column_order: [], column_id: [#0], pk_indices: [] } | |||
StreamProject { exprs: [Case(($0 = 1:Int32), 1:Int32::Decimal, ($0 = 2:Int32), 2:Int32::Decimal, Normalized(0.0):Decimal)], expr_alias: [ ] } | |||
StreamMaterialize { table_id: 0, column_order: [$1 ASC], column_id: [#0], pk_indices: [1] } |
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.
Maybe out of the scope of this PR. Is pk_indices
redundant with column_order
? In another words, is it guaranteed that column_order
must contain all PK columns with same order?
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.
yes... we can refactor it later
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.
#1208 also needs to be refactored.
Above code should be rewritten as an assertion.
What's changed and what's your intention?
due to mistake when resolve conflict, #1185 lose this part.
PLEASE DO NOT LEAVE THIS EMPTY !!!
Please explain IN DETAIL what the changes are in this PR and why they are needed:
Checklist
Refer to a related PR or issue link (optional)