Skip to content

Conversation

@dfabulich
Copy link

  1. Merged in Add $count_all_possible_rows parameter to doSearch iftechfoundation/ifdb#1261 as a fast-forward merge
  2. Merged in main
  3. Merged in Improve "IFDB Recommends" performance, adding $count_all_possible_rows parameter to doSearch iftechfoundation/ifdb#1265, resolving conflicts in favor of 1261
  4. Merged in Use the doSearch function for getting new games on the front page iftechfoundation/ifdb#1196

With that done, the resulting branch seemed to work, and I found that the home page queries for new games, new reviews, and IFDB recommends were using fast index queries with and without a custom filter set for -tag:sexual content.

We can merge this into filter-games-version-3 at any time. Eventually, I'll merge 1265 and then 1196, at which point iftechfoundation#1199 will only show the new stuff.

alice-blue and others added 30 commits December 4, 2024 14:06
`numRatingsInAvg` is what we display in search results. Querying a different number from what we display made the search results look wrong.

Fixes iftechfoundation#1252
…-newitems

Search results call it `devsys` instead of `system`
Prior to iftechfoundation#1256, we were excluding special reviews, embargoed reviews, and sandboxed reviews with a `where` clause, but this meant that some games were missing from `gameRatingsSandbox0`, when all of a game's reviews were special reviews. In iftechfoundation#1256, I removed the `where` clause and turned them into criteria on the `join` clauses.

This worked fine for special reviews and embargoed reviews, but it didn't work for sandboxed reviews. We did a left join to the `users` table, excluding sandboxed users there, but we didn't use the results of that join. We never used `users.userid` for anything, and we never excluded any sandboxed reviews in the criteria for the `reviews` table.

I've removed the join to `users` and replaced it with another criterion on the join to the `reviews` table, excluding reviews where the `userid` is sandboxed.

I've confirmed that this still excludes special and embargoed reviews, and now it excludes sandboxed reviews, too. Also, I've confirmed that the number of rows matches the number of rows in the games table, so every game has a matching row in `gameRatingsSandbox0`.

Fixes iftechfoundation#1263
…ndbox-gameratings

Correctly exclude sandboxed ratings from `gameRatingsSandbox0`
…-numRatingsInAvg

`#ratings:` search should use `numRatingsInAvg` not `numRatingsTotal`
This commit sets `$browse = 1`, which makes `searchutil.php` do an inner join to `gameRatingsSandbox0_mv`, allowing the query planner to grab the top N listings from the `starsort` index.
The query planner couldn't figure out how to handle joins with complex `on` filters, so I replaced them with `where gameid in` clauses for `played:` `willplay:` `wontplay:` `reviewed:` and `rated:` queries.
$maxpicks = 12; // Get the first twelve results. (We want extras so we're not always displaying the same games.)
$limit = "limit 0, $maxpicks";
$browse = 0;
$browse = 1;
Copy link
Owner

Choose a reason for hiding this comment

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

The reasoning for this seems obscure enough that it might be worth explaining in a comment

// table scan. If the total number of rows is not needed, we can skip
// `sql_calc_found_rows` to speed up the query.
$sql_calc_found_rows = "";
}
Copy link
Owner

Choose a reason for hiding this comment

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

I think 1123-1129 can be removed (this condition is accounted for in 1114)

Copy link
Owner

@alice-blue alice-blue Jan 22, 2025

Choose a reason for hiding this comment

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

After merging your PR, I did another commit to delete those lines in the filter games 3 branch

@alice-blue alice-blue merged commit 07c67d6 into alice-blue:filter-games-version-3 Jan 22, 2025
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.

2 participants