diff --git a/includes/query/class-tmdo-post-query-router.php b/includes/query/class-tmdo-post-query-router.php index 922277a..7e82377 100644 --- a/includes/query/class-tmdo-post-query-router.php +++ b/includes/query/class-tmdo-post-query-router.php @@ -778,22 +778,58 @@ class TMDO_Post_Query_Router { if ( empty( $copy ) ) { return $request; } + return self::ties_on_post_id( self::without_group_by( $request ), self::table_alias( $copy['post_type'], $copy['group'] ), '' ); + } + + /** + * ORDER BY terms that are posts.ID alone (with or without a direction) + * read as the equal flat post_id — in $direction, or each in its own when + * $direction is '' — and only once. posts.ID inside an expression a plugin + * added (FIELD(), IN …) is left alone: a direction inserted there breaks + * the SQL. + * + * @param string $request SELECT statement. + * @param string $alias Flat table alias. + * @param string $direction 'ASC', 'DESC' or '' (keep each term's own). + * @return string + */ + private static function ties_on_post_id( string $request, string $alias, string $direction ): string { global $wpdb; - $tie = '`' . self::table_alias( $copy['post_type'], $copy['group'] ) . '`.`post_id` '; - return (string) preg_replace_callback( - '/\bORDER BY\b(?!.*\bORDER BY\b).*$/s', - static function ( $m ) use ( $wpdb, $tie ) { - $order = (string) preg_replace_callback( - '/`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?(?!\w)(?:\s+(ASC|DESC)\b)?/i', - static fn( $id ) => $tie . ( 'DESC' === strtoupper( $id[1] ?? '' ) ? 'DESC' : 'ASC' ), - $m[0] - ); - return (string) preg_replace( '/(,\s*' . preg_quote( $tie, '/' ) . '(ASC|DESC))(\s*,\s*' . preg_quote( $tie, '/' ) . '\2)+/', '$1', $order ); - }, - self::without_group_by( $request ) + $tie = "`{$alias}`.`post_id` "; + $id = '`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?'; + return self::replaced( + preg_replace_callback( + '/\bORDER BY\b(?!.*\bORDER BY\b).*$/s', + static function ( $m ) use ( $id, $tie, $direction ) { + $order = self::replaced( + preg_replace_callback( + '/(?<=ORDER BY |,)(\s*)' . $id . '(?:\s+(ASC|DESC)\b)?(?=\s*(?:,|\bLIMIT\b|$))/i', + static fn( $t ) => $t[1] . $tie . ( '' !== $direction ? $direction : ( 'DESC' === strtoupper( $t[2] ?? '' ) ? 'DESC' : 'ASC' ) ), + $m[0] + ), + $m[0] + ); + return self::replaced( preg_replace( '/(,\s*' . preg_quote( $tie, '/' ) . '(ASC|DESC))(\s*,\s*' . preg_quote( $tie, '/' ) . '\2)+/', '$1', $order ), $order ); + }, + $request + ), + $request ); } + /** + * A preg_* result, or the text it was given when PCRE failed (backtrack + * or JIT stack limit on a very long request): the query stays unrouted + * in shape rather than empty. + * + * @param string|null $result preg_* return value. + * @param string $fallback Input text. + * @return string + */ + private static function replaced( ?string $result, string $fallback ): string { + return $result ?? $fallback; + } + /** * WordPress groups by posts.ID for any tax or meta query. With a flat row * (one per post) as the only join there is nothing to collapse, and the @@ -808,7 +844,7 @@ class TMDO_Post_Query_Router { if ( 1 !== preg_match_all( '/\bJOIN\b/i', $request ) ) { return $request; } - return (string) preg_replace( '/\s+GROUP BY\s+`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?(?=\s)/', ' ', $request, 1 ); + return self::replaced( preg_replace( '/\s+GROUP BY\s+`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?(?=\s)/', ' ', $request, 1 ), $request ); } /** @@ -882,16 +918,9 @@ class TMDO_Post_Query_Router { // post_id, once, in the sort's direction: a (column, post_id) index then // returns rows in order with no filesort. Ties are ordered by ID the same // way as the column (natively ID DESC); pagination stays stable. - $tie = "`{$alias}`.`post_id` " . ( 'ASC' === strtoupper( (string) $sort['order'] ) ? 'ASC' : 'DESC' ); - $flat_first = preg_replace_callback( - '/\bORDER BY\b(?!.*\bORDER BY\b).*$/s', - static function ( $m ) use ( $wpdb, $tie ) { - $order = (string) preg_replace( '/`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?(?!\w)(\s+(ASC|DESC)\b)?/i', $tie, $m[0] ); - return (string) preg_replace( '/(,\s*' . preg_quote( $tie, '/' ) . ')(\s*,\s*' . preg_quote( $tie, '/' ) . ')+/', '$1', $order ); - }, - (string) $flat_first - ); - return (string) preg_replace( '/^\s*SELECT\s/', 'SELECT STRAIGHT_JOIN ', (string) $flat_first, 1 ); + $direction = 'ASC' === strtoupper( (string) $sort['order'] ) ? 'ASC' : 'DESC'; + $flat_first = self::ties_on_post_id( $flat_first, $alias, $direction ); + return self::replaced( preg_replace( '/^\s*SELECT\s/', 'SELECT STRAIGHT_JOIN ', $flat_first, 1 ), $request ); } /** @@ -901,7 +930,7 @@ class TMDO_Post_Query_Router { * @return string */ private static function without_found_rows( string $request ): string { - return (string) preg_replace( '/\bSQL_CALC_FOUND_ROWS\s+/', '', $request, 1 ); + return self::replaced( preg_replace( '/\bSQL_CALC_FOUND_ROWS\s+/', '', $request, 1 ), $request ); } /** diff --git a/tests/integration/PostQueryRouterCopyTest.php b/tests/integration/PostQueryRouterCopyTest.php index 629b928..ccfc1b5 100644 --- a/tests/integration/PostQueryRouterCopyTest.php +++ b/tests/integration/PostQueryRouterCopyTest.php @@ -100,6 +100,13 @@ class PostQueryRouterCopyTest extends TestCase { $this->assertStringEndsWith( 'ORDER BY ' . self::ALIAS . '.`post_date` DESC LIMIT 0, 10', $this->request( '', '' ) ); } + public function test_copy_route_leaves_posts_id_inside_expressions_alone(): void { + global $wpdb; + $sql = $this->request( '', ', FIELD({posts}.ID,5,9) DESC, ({posts}.ID IN (3,4)) DESC, {posts}.ID DESC' ); + + $this->assertStringEndsWith( "`post_date` DESC, FIELD({$wpdb->posts}.ID,5,9) DESC, ({$wpdb->posts}.ID IN (3,4)) DESC, " . self::ALIAS . '.`post_id` DESC LIMIT 0, 10', $sql ); + } + public function test_copy_route_keeps_group_by_with_another_join(): void { global $wpdb; $sql = $this->request( " LEFT JOIN wp_term_relationships ON ({$wpdb->posts}.ID = wp_term_relationships.object_id)" ); diff --git a/tests/integration/PostQueryRouterFoundRowsTest.php b/tests/integration/PostQueryRouterFoundRowsTest.php index 3dd49db..661f12e 100644 --- a/tests/integration/PostQueryRouterFoundRowsTest.php +++ b/tests/integration/PostQueryRouterFoundRowsTest.php @@ -272,6 +272,25 @@ class PostQueryRouterFoundRowsTest extends TestCase { $this->assertSame( array( '2', '3' ), $wpdb->get_col( $sql ) ); } + /** + * Only standalone posts.ID terms are tie-breaks: inside an expression a + * plugin added (pinning posts with FIELD() or IN) the direction must not be + * inserted, or the SQL breaks (2.4 correctness review). + */ + public function test_flat_first_leaves_posts_id_inside_expressions_alone(): void { + global $wpdb; + list( $router, $query, $request ) = $this->sorted(); + $pinned = str_replace( ' LIMIT 0, 10', ", FIELD({$wpdb->posts}.ID,3,2) DESC, ({$wpdb->posts}.ID IN (2)) DESC LIMIT 0, 10", $request ); + + $sql = $router->posts_request( $pinned, $query ); + + $this->assertStringStartsWith( 'SELECT STRAIGHT_JOIN ', $sql ); + $this->assertStringContainsString( "FIELD({$wpdb->posts}.ID,3,2) DESC, ({$wpdb->posts}.ID IN (2)) DESC LIMIT", $sql ); + $wpdb->last_error = ''; + $this->assertSame( $wpdb->get_col( str_replace( 'SQL_CALC_FOUND_ROWS ', '', $pinned ) ), $wpdb->get_col( $sql ) ); + $this->assertSame( '', $wpdb->last_error ); + } + public function test_date_ordered_filtered_request_keeps_wordpress_join_order(): void { list( $router, $query, $request ) = $this->routed();