Use semver_ord column for sorting versions
#10819
Conversation
This allows us to use `String` in seek variants. The `.clone()` call isn't ideal, and shouldn't be necessary, but rewriting the `seek!()` macro was out of scope for this effort.
… in the database
Turbo87
added
C-enhancement ✨
| // A decodable seek value, WyIwLjAuMCIsMTAwXQ (["0.0.0",100]), but doesn't actually exist | ||
| let json: VersionList = anon | ||
| .get_with_query(url, "per_page=10&sort=semver&seek=MTAwCg") | ||
| .get_with_query(url, "per_page=10&sort=semver&seek=WyIwLjAuMCIsMTAwXQ") |
There was a problem hiding this comment.
with the old link the API will now return a "400 Bad Request" response
|
Ah, I just reviewed an outdated version 😅 I guess you decided not to introduce the change to release_tracks in this PR after all? |
not sure what happened, but I only pushed this branch once so far 😅
AFAIK I didn't touch the |
There was a problem hiding this comment.
I was tempted to just approve this without even reviewing it, because:
But I promise I did review it.
I was a bit nervous about the seek behaviour change, so I decided to dig through the last week's worth of logs to see if we were getting any significant programmatic use of the seek parameter with sort=semver, and the answer is: as far as I can tell, we have literally zero. There were seven requests in the last week with sort=semver and a non-empty seek parameter: two of them were me just now trying to remember how that page worked, and the other five were all from real browsers that I have no reason to believe weren't actually real people clicking on the load more button.
So I think we're good.
I'm not sure either. I just saw an "outdated" label on my previously deleted input 🤷
I think a separate PR is fine, but it's also okay if you'd like to make the changes in this PR, since at least part of it would be related to |
thanks! 😆
I think a separate PR makes more sense. since I'll be on vacation starting tomorrow, that will have to wait until after my vacation though. feel free to take over though, if you want :) |
3 participants
This PR changes how we return sorted versions from the
GET /api/v1/crates/{name}/versionsAPI endpoint.Previously, we needed to download all version numbers from the database to the API server and perform the sorting and filtering there, since the database did not support sorting by semantic version. With #10763 that changed, so now we can perform the sorting and filtering directly in the database, which simplifies the code that the API server needs to run and the data that needs to be transferred between the API server and the database.
Note that I have not added any database indexes for the
semver_ordcolumn yet, so there is potentially some more room for performance improvements.The implementation as-is has a small caveat, in that it is a breaking change to the
seekquery parameter format. We could potentially build a compatibility layer to make oldnext_pagelinks still work with the new implementation, but since seek pagination for this endpoint is rarely used outside of our own frontend, I'm not sure whether it's really worth it for the short transition period between the two implementations.Related:
semver_ord(num)SQL function andversion.semver_ordcolumn #10763