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 gameId column to LeagueScoreJournal #772

Merged
merged 4 commits into from
Oct 18, 2023
Merged

Conversation

BlackYps
Copy link
Contributor

Requires league service 1.5.0

With this the loginId and gameId just exist as integers. They don't have a relationship to the actual object with that Id. How can this be added given that we would cross databases with this?

@codecov
Copy link

codecov bot commented Sep 2, 2023

Codecov Report

Merging #772 (ec4653f) into develop (0d570f5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop     #772   +/-   ##
==========================================
  Coverage      78.01%   78.02%           
- Complexity      1262     1263    +1     
==========================================
  Files            249      249           
  Lines           3812     3813    +1     
  Branches         241      241           
==========================================
+ Hits            2974     2975    +1     
  Misses           738      738           
  Partials         100      100           
Files Changed Coverage Δ
...aforever/api/league/domain/LeagueScoreJournal.java 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d570f5...ec4653f. Read the comment docs.

Copy link
Member

@bukajsytlos bukajsytlos left a comment

Choose a reason for hiding this comment

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

lgtm

@Brutus5000
Copy link
Member

They don't have a relationship to the actual object with that Id. How can this be added given that we would cross databases with this?

To our knowledge you can't and have to do 2 queries instead. I raised the question in the Elide Discord though. Although I recently discovered in MySQL you can cross join databases if you prefix the tables with the db name. Not sure if this is supported by Hibernate, then we could just drop the Multiplex store altogether...

@Brutus5000 Brutus5000 merged commit b1e7fca into develop Oct 18, 2023
5 checks passed
@Brutus5000 Brutus5000 deleted the league-replay-id branch October 18, 2023 18:26
@BlackYps
Copy link
Contributor Author

Is doing two queries still the way to go?

@Brutus5000
Copy link
Member

Currently yes, but I found a way that MySQL can do crossover joins if a user actually has permissions for both tables.
So we could actually drop the 2nd elide datastore and then we could do regular elide joins again. But time time time. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants