-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Reduce usage of allcols on update #2596
Conversation
Test fails for this |
770e9e3
to
5ed36ba
Compare
Codecov Report
@@ Coverage Diff @@
## master #2596 +/- ##
==========================================
+ Coverage 27.31% 27.32% +<.01%
==========================================
Files 86 86
Lines 17146 17143 -3
==========================================
Hits 4684 4684
+ Misses 11784 11781 -3
Partials 678 678
Continue to review full report at Codecov.
|
@lafriks done. |
LGTM |
LGTM Out of curiosity, what is the main benefit of only updating affected columns? Performance, or something else? |
@ethantkoenig I think there are two reasons. One is for performance, another is to reduce the possibility of wrong updates. I remember the random avatar bug is affected by updating all the columns when login. |
Reduce some unneccessary of
AllCols
on update.