Adding WHERE clause to redirection query overwrites existing query

Thanks for the response, Pratic, but I’m not sure I follow.

As you noted, the $table->where() call should append the where clause, however in this case it is not.

The Pastebin I provided should be independently replicable (tested against WP v6.2 + RM v1.0.116) – just add it to your test env’s functions.php. You can see the where() call on line 22 (the fourth parameter is unset in this example, since it defaults to ‘AND’ which is the behavior we want here. In the IRL code linked below, ‘AND’ is explicitly set.).

The real-world code can be found in this PR. This method hooks onto the rank_math/redirection/get_redirections_query action to append the necessary where clause (i.e only the results after a provided post ). However, instead of appending the clause, it replaces it, as evidenced by this test (because the preexisitng order/orderby clauses are overwritten, only 1 redirection is returned). But again, the Pastebin replicates the issue without all that extraneous logic.

If there’s any additional context/debugging information you need, please let me know.

Thanks Pratik, the pastebin you provided is working in isolation, and I am working to apply it to the actual code. Leaving this open for now and will post back if I run into any issues.

(PS just to bring it to the attention of the dev team: the overuse of shared static properties and modifying references instead of instances really makes it a PITA for developers to integrate, test, and build with RankMath. The lack of encapsulation around DB::get_redirection_by_id() is just one example.)

Looks like I am running into the same issue calling DB::get_redirections( $args ); multiple times in the same process. The where and order_by args in the new query get appended on the previous $table (since its a static singleton), instead of the $table getting reset between calls.

For example, calling that method three times, leaves me with
SELECT SQL_CALC_FOUND_ROWS * FROM wp_rank_math_redirections WHERE status = 'active' AND status = 'active' AND status = 'active' AND created < '2023-06-11 11:12:46' ORDER BY created, created, created LIMIT 0, 3.

I’m not seeing any publicly accessible reset methods, and unlike your workaround for querying a single ID, manually building a SQL query that mimics DB::get_redirections() isn’t really tenable if we want our extension to be future-compatible with Rank Math.

It’s a fairly standard use case when using WPGraphQL APIs (e.g. in the same request you might get the list of active redirections, the redirection for a given id, and any redirections that match a provided uri).

It’s also a performance necessity when querying for _lots_ of items. This is less critical for redirects but it looks like 404 logs use the same static patterns.

And sadly, there’s no real way to ‘workaround’ this in the frontend. Static site generation (Gatsby, NextJS + Apollo, etc) will combine and dedupe queries to minimize the number of requests, and paginate through results to avoid killing the database.

(PS, we had the same issue with RankMath:Paper, which was resolved by adding a public static reset() method. It’s not ideal – there or here – but works in a pinch a lot easier to implement than refactoring Rank Math to be DRY and use dependency injection)