-
Notifications
You must be signed in to change notification settings - Fork 526
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
chore(frontend): distribute index by arrange key #2056
Conversation
Signed-off-by: Alex Chi <iskyzh@gmail.com>
Codecov Report
@@ Coverage Diff @@
## main #2056 +/- ##
==========================================
- Coverage 70.95% 70.95% -0.01%
==========================================
Files 633 633
Lines 81175 81192 +17
==========================================
+ Hits 57601 57609 +8
- Misses 23574 23583 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
) -> Result<Self> { | ||
// ensure the same pk will not shuffle to different node | ||
let input = match input.distribution() { | ||
Distribution::Single => input, | ||
_ => Distribution::HashShard(input.pk_indices().to_vec()) | ||
.enforce_if_not_satisfies(input, Order::any()), | ||
_ => Distribution::HashShard(if distribute_only_order_by { |
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.
Looks somehow strange to me... I think this should be handled by the caller, not 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.
Infer distribution is always done inside operators?
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.
Hmmm. Also makes sense
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.
In fact I'm also considering making Materialize
to be AnyShard
, and let the create mv / index statement to decide the distribution by specifying it in PlanRoot. But in this case, we might need some redesign.
e2e_test/v2/streaming/index.slt
Outdated
# TODO: support drop index | ||
|
||
statement ok | ||
drop index iii_index_t1; | ||
drop materialized view iii_index_t1; |
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.
May let drop index
act as an alias of drop materialized view
in the future
Signed-off-by: Alex Chi iskyzh@gmail.com
What's changed and what's your intention?
As title
Checklist
Refer to a related PR or issue link (optional)
close #2017