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

Make game searches do an inner join to gameRatingsSandbox0_mv #1273

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dfabulich
Copy link
Collaborator

The code was doing a left join (aka a left outer join) to gameRatingsSandbox0_mv unless $browse mode is enabled and you're using one of the specified sort orders, in which case it does a simple join (an inner join).

I wrote this code in PR #270, specifically in 3bd2c7a. But… why did I write that? Why didn't I just do an inner join in all cases?

To be quite honest, I can't remember. I bet the issue was that gameRatingsSandbox0_mv didn't have exactly as many rows as the games table. (And I bet I was misusing $browse … it probably should have been !$term.)

I believe that gameRatingsSandbox0_mv will always have a row for every game in the games table, now that we've merged #1266, and so it should be safe to do an inner join in all cases.

The code was doing a `left join` (aka a `left outer join`) to `gameRatingsSandbox0_mv` unless `$browse` mode is enabled and you're using one of the specified sort orders, in which case it does a simple `join` (an `inner join`).

I wrote this code in PR #270, specifically in 3bd2c7a. But… why did I write that? Why didn't I just do an inner join in all cases?

To be quite honest, I can't remember. I bet the issue was that `gameRatingsSandbox0_mv` didn't have exactly as many rows as the `games` table. (And I bet I was misusing `$browse` … it probably should have been `!$term`.)

I believe that `gameRatingsSandbox0_mv` will always have a row for every game in the games table, now that we've merged #1266, and so it should be safe to do an inner join in all cases.
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.

1 participant