fix: uniq by hash, instead of transaction #2062
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
zachdaniel
force-pushed
the
remove-duplicate-token-transfers
branch
from
660b913
to
5edf4b4
Compare
zachdaniel
commented
May 29, 2019
Pull Request Test Coverage Report for Build aa3b38e8-c50b-40de-bc14-3244901e8888
💛 - Coveralls |
zachdaniel
force-pushed
the
remove-duplicate-token-transfers
branch
from
5edf4b4
to
c7be577
Compare
zachdaniel
commented
May 29, 2019
| [address_bytes, block_number, page_size] | ||
| ) | ||
| defp transaction_hashes_from_token_transfers_sql(address_bytes, %PagingOptions{page_size: page_size} = paging_options) do | ||
| query = |
There was a problem hiding this comment.
This doesn't need to be changed, but while debugging this issue I cleaned up this query. Might as well use the new more legible version. I will remove this change on request.
zachdaniel
force-pushed
the
remove-duplicate-token-transfers
branch
from
c7be577
to
77e8f45
Compare
ayrat555
approved these changes
May 30, 2019
I can reproduce this issue in stg. I will test this fix with the new wave of updates to stg. |
pasqu4le
approved these changes
May 30, 2019
I hurried with this statement. I didn't find an example on stg. I will see this fix live on Sokol. |
vbaranov
approved these changes
May 30, 2019
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.
#883
Bug Fixes
when deduplicating transactions, don't compare the entire object, because preloaded associations could have the list objects in a different order. Instead, deduplicate them using their hash.
CHANGELOG.mdwith this PRI tried really hard to reproduce this in a test, but the problem is that it is an inconsistent behavior. Postgres chooses the order of the returned rows. I could sort them to ensure that they always come back in the same order, but that is not actually a requirement of the UI, so we would be slowing it down just to be able to test it. I don't mind adding the sort for the sake of the test, just let me know.
I'm pretty positive that this is the problem but I haven't been able to find a local occurrence of this myself so we should test it against the sokol instance that had the original issue reported.