diff --git a/includes/engine/class-tmdo-entity-registry.php b/includes/engine/class-tmdo-entity-registry.php index 6e4cdfe..cc86021 100644 --- a/includes/engine/class-tmdo-entity-registry.php +++ b/includes/engine/class-tmdo-entity-registry.php @@ -159,9 +159,12 @@ final class TMDO_Entity_Registry { self::$scopes[ $entity_type ][ $group_name ] = $post_types; } $keys = array_column( $normalized, 'key' ); + // A sort index may end with the entity ID (flat-first tie-break, 2.4 PR-118). + $id_col = (string) self::$adapters[ $entity_type ]->get_entity_id_column(); foreach ( (array) ( $args['indexes'] ?? array() ) as $index_name => $columns ) { $columns = array_values( array_map( 'strval', (array) $columns ) ); - if ( $columns && ! array_diff( $columns, $keys ) ) { + $fields = '' !== $id_col && end( $columns ) === $id_col ? array_slice( $columns, 0, -1 ) : $columns; + if ( $fields && ! array_diff( $fields, $keys ) ) { self::$indexes[ $entity_type ][ $group_name ][ (string) $index_name ] = $columns; } } diff --git a/includes/query/class-tmdo-post-query-router.php b/includes/query/class-tmdo-post-query-router.php index ede1648..443505e 100644 --- a/includes/query/class-tmdo-post-query-router.php +++ b/includes/query/class-tmdo-post-query-router.php @@ -797,10 +797,17 @@ class TMDO_Post_Query_Router { if ( 1 === preg_match_all( '/\bJOIN\b/i', (string) $flat_first ) ) { $flat_first = preg_replace( '/\s+GROUP BY\s+`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?(?=\s)/', ' ', (string) $flat_first, 1 ); } - // Tie-breaks on posts.ID (ours, and any a plugin appended) read the flat post_id. + // Tie-breaks on posts.ID (ours, and any a plugin appended) read the flat + // 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 fn( $m ) => preg_replace( '/`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?(?!\w)/', "`{$alias}`.`post_id`", $m[0] ), + 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 ); diff --git a/tests/integration/PostQueryRouterFoundRowsTest.php b/tests/integration/PostQueryRouterFoundRowsTest.php index a806a54..3dd49db 100644 --- a/tests/integration/PostQueryRouterFoundRowsTest.php +++ b/tests/integration/PostQueryRouterFoundRowsTest.php @@ -226,14 +226,14 @@ class PostQueryRouterFoundRowsTest extends TestCase { * * @return array{0:TMDO_Post_Query_Router, 1:WP_Query, 2:string} */ - private function sorted( string $compare = '>=' ): array { + private function sorted( string $compare = '>=', string $order = 'DESC' ): array { global $wpdb; $query = new WP_Query(); $query->set( 'post_type', 'fr_item' ); $query->set( 'post_status', 'publish' ); $query->set( 'meta_query', array( 'price' => array( 'key' => 'fr_price', 'value' => 15, 'compare' => $compare, 'type' => 'NUMERIC' ) ) ); $query->set( 'orderby', 'price' ); - $query->set( 'order', 'DESC' ); + $query->set( 'order', $order ); $router = new TMDO_Post_Query_Router(); $router->pre_get_posts( $query ); @@ -252,11 +252,26 @@ class PostQueryRouterFoundRowsTest extends TestCase { $this->assertStringStartsWith( 'SELECT STRAIGHT_JOIN ', $sql ); $this->assertStringContainsString( 'FROM `' . self::TABLE . '` AS `wpdoflat_fr_item_fr_core` INNER JOIN ' . $wpdb->posts . ' ON', $sql ); $this->assertStringNotContainsString( 'LEFT JOIN `' . self::TABLE . '`', $sql ); - $this->assertStringEndsWith( 'ORDER BY `wpdoflat_fr_item_fr_core`.`fr_price` DESC, `wpdoflat_fr_item_fr_core`.`post_id` DESC, `wpdoflat_fr_item_fr_core`.`post_id` DESC LIMIT 0, 10', $sql, 'posts.ID tie-breaks read the flat post_id' ); + $this->assertStringEndsWith( 'ORDER BY `wpdoflat_fr_item_fr_core`.`fr_price` DESC, `wpdoflat_fr_item_fr_core`.`post_id` DESC LIMIT 0, 10', $sql, 'posts.ID tie-breaks read the flat post_id, once' ); $this->assertSame( array( '3', '2' ), $wpdb->get_col( $sql ) ); $this->assertSame( $wpdb->get_col( str_replace( 'SQL_CALC_FOUND_ROWS ', '', $request ) ), $wpdb->get_col( $sql ) ); } + /** + * The ID tie-break follows the sort direction, so a (column, post_id) + * index returns rows in order with no filesort (2.4 PR-118); ties are + * ordered by ID the same way as the column. + */ + public function test_flat_first_tie_break_follows_the_sort_direction(): void { + global $wpdb; + list( $router, $query, $request ) = $this->sorted( '>=', 'ASC' ); + + $sql = $router->posts_request( $request, $query ); + + $this->assertStringEndsWith( 'ORDER BY `wpdoflat_fr_item_fr_core`.`fr_price` ASC, `wpdoflat_fr_item_fr_core`.`post_id` ASC LIMIT 0, 10', $sql ); + $this->assertSame( array( '2', '3' ), $wpdb->get_col( $sql ) ); + } + public function test_date_ordered_filtered_request_keeps_wordpress_join_order(): void { list( $router, $query, $request ) = $this->routed(); diff --git a/tests/unit/EntityRegistryIndexesTest.php b/tests/unit/EntityRegistryIndexesTest.php index cb4a399..02d4103 100644 --- a/tests/unit/EntityRegistryIndexesTest.php +++ b/tests/unit/EntityRegistryIndexesTest.php @@ -14,7 +14,9 @@ class EntityRegistryIndexesTest extends TestCase { protected function setUp(): void { TMDO_Entity_Registry::init(); - TMDO_Entity_Registry::register_adapter( 'post', $this->createMock( TMDO_Entity_Adapter_Interface::class ) ); + $adapter = $this->createMock( TMDO_Entity_Adapter_Interface::class ); + $adapter->method( 'get_entity_id_column' )->willReturn( 'post_id' ); + TMDO_Entity_Registry::register_adapter( 'post', $adapter ); } private function fields(): array { @@ -37,6 +39,16 @@ class EntityRegistryIndexesTest extends TestCase { $this->assertSame( array( 'ok' => array( 'x_price' ) ), TMDO_Entity_Registry::get_group_indexes( 'post', 'x_core' ) ); } + /** + * A sort index ends with the entity ID, so a flat-first search ordered by a + * column with the ID tie-break reads the index in order (2.4 PR-118). + */ + public function test_an_index_may_end_with_the_entity_id_column(): void { + TMDO_Entity_Registry::register_group( 'post', 'x_core', $this->fields(), array( 'indexes' => array( 'price_id' => array( 'x_price', 'post_id' ), 'id_first' => array( 'post_id', 'x_price' ) ) ) ); + + $this->assertSame( array( 'price_id' => array( 'x_price', 'post_id' ) ), TMDO_Entity_Registry::get_group_indexes( 'post', 'x_core' ), 'the ID only as the last column' ); + } + public function test_indexes_leave_the_field_definition_and_hash_alone(): void { TMDO_Entity_Registry::register_group( 'post', 'plain', $this->fields() ); TMDO_Entity_Registry::register_group( 'post', 'indexed', $this->fields(), array( 'indexes' => array( 'flag_price' => array( 'x_flag', 'x_price' ) ) ) );