chore: enable correct_pagination_assets_after_creating_new_one test #5192
Conversation
Signed-off-by: Lohachov Mykhailo <lohachov@soramitsu.co.jp>
aoyako
requested review from
s8sato,
mversic,
SamHSmith,
outoftardis,
0x009922,
DCNick3 and
dima74
as code owners
| // FIXME transaction is rejected for more than a certain number of instructions | ||
| const N_ASSETS: usize = 12; | ||
| const N_ASSETS: usize = 9; |
There was a problem hiding this comment.
I think this test has failed for some other reason than the FIXME item. I see the same result by iterating over submit_blocking and there are no transaction rejections due to too many instructions in submit_all_blocking. In my local 10 and 11 are the threshold of whether it passes or not
There was a problem hiding this comment.
what's the default fetch size, 10? this might be related to result batching
There was a problem hiding this comment.
The test fails due to the lexicographical sorting of strings ("1" -> "10" -> "11", not "1" -> "2" -> "3").
All of the sorting tests have this limit of 10 because they will also fail if the number is increased.
This test should verify that adding an asset in the middle should not break the ordering, which was not checked in the previous version.
There was a problem hiding this comment.
So the issue would be that metadata leaf values are implicitly transmuted from numbers to strings
There was a problem hiding this comment.
Yes, since the metadata value is a json string. I'm not sure if we have a functionality to specify a mapping/key function for sorting. Isn't the purpose of having json string to generalize metadata and defer custom processing to the user?
There was a problem hiding this comment.
Since PartialOrd and Ord cannot be naturally derived from serde_json::Value, we could:
- totally discard the concept of sorting metadata value
- originally define order between
Valuevariants
There was a problem hiding this comment.
Since PartialOrd and Ord cannot be naturally derived from serde_json::Value, we could:
good point. My previous comment should be disregarded. Now I think this is the way to go:
totally discard the concept of sorting metadata value
There was a problem hiding this comment.
If we go this way, the problem then is currently we are sorting query results only by metadata
There was a problem hiding this comment.
this is where it all started. But since then we moved to untyped metadata value. Maybe it would be good to provide a sorting function? The comparison function will get untyped metadata, parse it and then compare the objects
There was a problem hiding this comment.
Yes, there seems to be a trade-off between keeping metadata values untyped and serving query sorting on the peer side
Resolves #4795
Checklist
CONTRIBUTING.md.