Fix token_account primary index #1424
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Nana-EC <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1424 +/- ##
============================================
+ Coverage 85.07% 85.36% +0.28%
- Complexity 359 364 +5
============================================
Files 257 257
Lines 6367 6367
Branches 689 689
============================================
+ Hits 5417 5435 +18
+ Misses 722 707 -15
+ Partials 228 225 -3
Continue to review full report at Codecov.
|
Signed-off-by: Nana-EC <[email protected]>
steven-sheehy
requested changes
Jan 8, 2021
| @@ -79,7 +79,7 @@ alter table token | |||
| set (timescaledb.compress, timescaledb.compress_segmentby = 'token_id'); | |||
|
|
|||
| alter table token_account | |||
| set (timescaledb.compress, timescaledb.compress_segmentby = 'account_id'); | |||
| set (timescaledb.compress, timescaledb.compress_segmentby = 'account_id, token_id'); | |||
There was a problem hiding this comment.
Recommendation is to put the non-timestamp fields from your primary keys in the segmentby. Think we should remove account_id.
There was a problem hiding this comment.
Yes, with updated primary key makes sense for only token_id
Signed-off-by: Nana-EC <[email protected]>
steven-sheehy
previously approved these changes
Jan 8, 2021
Signed-off-by: Nana-EC <[email protected]>
steven-sheehy
approved these changes
Jan 8, 2021
|
Kudos, SonarCloud Quality Gate passed!
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Detailed description:
#1364 updated the token_account table to have a primary index on the timestamp.
However, an account can be associated with multiple tokens in a single TokenAssociate transaction.
V1.33.0__drop_token_account_id.sqlmigration file to settoken_accountprimary index to usecreated_timestamp, token_idV2.0.2__time_scale_index_init.sqlfile to settoken_accountprimary index to usecreated_timestamp, token_idTokenAccountRepositoryTestwith test to capture scenarioWhich issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist