-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
[SQL Lab] Fix TableSelector perf for large option sets #7868
[SQL Lab] Fix TableSelector perf for large option sets #7868
Conversation
4f7fe94
to
0bffc15
Compare
placeholder={t('Select table or type table name')} | ||
autosize={false} | ||
onChange={this.changeTable} | ||
filterOptions={this.state.filterOptions} |
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.
there is a unit test for filterOptions prop. are you sure to remove it?
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.
oops, fixed!
Does this get rid of autocomplete? |
@khtruong nope, autocomplete still works as before. |
0bffc15
to
28f591a
Compare
Codecov Report
@@ Coverage Diff @@
## master #7868 +/- ##
==========================================
+ Coverage 65.8% 65.83% +0.02%
==========================================
Files 461 461
Lines 22210 22210
Branches 2425 2425
==========================================
+ Hits 14616 14621 +5
+ Misses 7473 7468 -5
Partials 121 121
Continue to review full report at Codecov.
|
CATEGORY
Choose one
SUMMARY
If you have a large number of tables with a single schema (we have 28k in one of ours), it would take about 10 seconds to load the table selector. This was due to generating the search index, tokenizing, and doing other search-y related things. It turns out that getting rid of the indexing and adding
ignoreAccents={false}
(the same thing as #7791) improves the loading speed (down to under half a second) without hurting the search speed.TEST PLAN
ADDITIONAL INFORMATION
REVIEWERS
@graceguo-supercat @michellethomas @khtruong @mistercrunch