Skip to content
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

Add support for retrieving submit/rank date from local metadata cache in version 2 #29553

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Aug 21, 2024

As is this will not retroactively fix beatmaps where this is missing, although in theory I could write a background processing job that does it using this code. Will do on request, I suppose.

Compatibility matrix:

online.db v1 online.db v2
clients before this PR 🟠1 🟠2
clients after this PR 🟠1 🟢3

Footnotes

  1. Lookups using the local metadata will succeed, but submit and rank date will not be populated. 2

  2. Lookups using the local metadata will succeed despite the schema changes to online.db, because they happen to be backwards-compatible; the columns removed in https://github.com/ppy/osu-onlinedb-generator/commit/da5a35b419c83ee681e97b2132c5f57cc1d32fc9 were not used.

  3. Lookups using the local metadata will succeed, and contain submit and rank date.

@bdach bdach added the area:import dealing with importing of data from stable or file formats label Aug 21, 2024
Comment on lines +267 to +272
"""
SELECT `b`.`beatmapset_id`, `b`.`beatmap_id`, `b`.`approved`, `b`.`user_id`, `b`.`checksum`, `b`.`last_update`, `s`.`submit_date`, `s`.`approved_date`
FROM `osu_beatmaps` AS `b`
JOIN `osu_beatmapsets` AS `s` ON `s`.`beatmapset_id` = `b`.`beatmapset_id`
WHERE `b`.`checksum` = @MD5Hash OR `b`.`beatmap_id` = @OnlineID OR `b`.`filename` = @Path
""";
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all I've ever wanted. Fixes all the issues with @"" quite well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I'll point out which is more relevant for server code, is that this still puts lines on a new line. I've been told in the past the reason we use "x" \n + "y" is because it's sometimes filtered on via proxysql and that makes things easier somehow.

Otherwise I'm all for this syntax.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, generally when laying out like this I want the newlines in the SQL for logging purposes too. So this would always be something I consider during review.

@peppy peppy self-requested a review August 22, 2024 05:20
@peppy
Copy link
Member

peppy commented Aug 22, 2024

Code looks sane enough.

@bdach did you do something silly like create a local online.db to test this? if you did, can you provide that here? if not i'll probably get this deployed and force a run immediately.

@bdach
Copy link
Collaborator Author

bdach commented Aug 22, 2024

Here's one to test with from my local osu-web env: online.zip

Recommend testing with something like https://osu.ppy.sh/beatmapsets/386151.

@peppy
Copy link
Member

peppy commented Aug 22, 2024

Checks out.

I'll deploy a new database later today.

@peppy peppy merged commit 066c34e into ppy:master Aug 22, 2024
9 of 14 checks passed
@bdach bdach deleted the local-cache-updates branch August 22, 2024 06:06
bdach added a commit to bdach/osu that referenced this pull request Sep 13, 2024
…cache

People keep asking why ppy#29553 didn't fix
their databases (as stated in the PR, it didn't intend to), so this
should do it for them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:import dealing with importing of data from stable or file formats size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Beatmap imports using the local cache lookup do not get a submitted / ranked date populated
3 participants