-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
LookupVindex: Multiple lookup tables support for LookupVindexCreate
#17566
base: main
Are you sure you want to change the base?
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Noble Mittal <[email protected]>
d51393b
to
c9e74bf
Compare
Signed-off-by: Noble Mittal <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17566 +/- ##
==========================================
- Coverage 67.69% 67.61% -0.09%
==========================================
Files 1584 1585 +1
Lines 254541 255140 +599
==========================================
+ Hits 172315 172509 +194
- Misses 82226 82631 +405 ☔ View full report in Codecov by Sentry. |
{ | ||
description: "dup vindex", | ||
input: &vschemapb.Keyspace{ | ||
Vindexes: map[string]*vschemapb.Vindex{ | ||
"v1": { | ||
Type: "xxhash", | ||
}, | ||
"v2": { | ||
Type: "xxhash", | ||
}, | ||
}, | ||
}, | ||
err: "only one vindex must be specified", |
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.
Since we will support backfilling multiple lookup tables, this unit test can be removed.
Signed-off-by: Noble Mittal <[email protected]>
Description
Testing
LookupVindex
command:vindex_params.json
file:Related Issue(s)
Checklist
Deployment Notes