diff --git a/includes/query/class-tmdo-post-query-router.php b/includes/query/class-tmdo-post-query-router.php index 88914b7..8db77f7 100644 --- a/includes/query/class-tmdo-post-query-router.php +++ b/includes/query/class-tmdo-post-query-router.php @@ -16,16 +16,18 @@ * Hook order: * 1. pre_get_posts — strip registered keys from meta_query, store in * $query->set( 'wpdo_post_clauses', $routed ) - * 2. posts_join — LEFT JOIN wp_wpdo_post_ per routed post_type + * 2. posts_join — LEFT JOIN wp_wpdo_post_ per routed group * 3. posts_where — append WHERE conditions targeting flat columns - * 4. posts_groupby — ensure GROUP BY posts.ID to dedup join multiplication + * + * Flat tables hold one row per post (UNIQUE post_id), so the joins never + * multiply rows and no GROUP BY is added. * * Status / date on the flat table (O-1): when the group's post_status / * post_date copies are ready (TMDO_Post_Status_Sync) and none is missing, * a single-post-type search with explicit statuses and date ordering - * INNER JOINs the table, filters the copies, orders by the copied - * post_date and skips GROUP BY (one row per post), so MySQL can walk - * `idx_post_status_date` and stop at the LIMIT. + * INNER JOINs the table, filters the copies and orders by the copied + * post_date, so MySQL can walk `idx_post_status_date` and stop at the + * LIMIT. Not with NOT EXISTS: posts without a flat row must still match. * * Found rows (O-4): routed searches drop SQL_CALC_FOUND_ROWS (it counts every * match inside the page query); found_posts_query answers from a COUNT of the @@ -85,7 +87,6 @@ class TMDO_Post_Query_Router { add_action( 'pre_get_posts', array( $this, 'pre_get_posts' ), 1000, 1 ); add_filter( 'posts_join', array( $this, 'posts_join' ), 10, 2 ); add_filter( 'posts_where', array( $this, 'posts_where' ), 10, 2 ); - add_filter( 'posts_groupby', array( $this, 'posts_groupby' ), 10, 2 ); add_filter( 'posts_orderby', array( $this, 'posts_orderby' ), 10, 2 ); add_filter( 'posts_request', array( $this, 'posts_request' ), 10, 2 ); add_filter( 'found_posts_query', array( $this, 'found_posts_query' ), 10, 2 ); @@ -175,7 +176,7 @@ class TMDO_Post_Query_Router { } /** - * LEFT JOIN flat tables for each routed post_type. + * LEFT JOIN the flat table of each routed group. * * @param string|null $join Current JOIN SQL. * @param \WP_Query $query Current WP_Query. @@ -191,23 +192,20 @@ class TMDO_Post_Query_Router { global $wpdb; foreach ( $routed as $post_type => $clauses ) { - // Each routed clause carries its group; use the first clause's group - // (all clauses for a given post_type currently target one group). - $group = $clauses[0]['group'] ?? ''; - if ( '' === $group ) { - continue; - } - $table = $wpdb->prefix . 'wpdo_post_' . sanitize_key( $group ); - $alias = self::table_alias( $post_type, $group ); + foreach ( array_unique( array_column( $clauses, 'group' ) ) as $group ) { + $table = $wpdb->prefix . 'wpdo_post_' . sanitize_key( $group ); + $alias = self::table_alias( $post_type, $group ); - // Don't double-join. - if ( false !== strpos( $join, "`{$alias}`" ) ) { - continue; - } + // Don't double-join. + if ( false !== strpos( $join, "`{$alias}`" ) ) { + continue; + } - $type = self::copy_route_for( $query, $post_type ) ? 'INNER' : 'LEFT'; - $join .= " {$type} JOIN `{$table}` AS `{$alias}` " - . "ON (`{$wpdb->posts}`.`ID` = `{$alias}`.`post_id`)"; + // The copy route only applies to single-group searches. + $type = self::copy_route_for( $query, $post_type ) ? 'INNER' : 'LEFT'; + $join .= " {$type} JOIN `{$table}` AS `{$alias}` " + . "ON (`{$wpdb->posts}`.`ID` = `{$alias}`.`post_id`)"; + } } return $join; @@ -265,28 +263,6 @@ class TMDO_Post_Query_Router { return '`' . self::table_alias( $copy['post_type'], $copy['group'] ) . '`.`post_date` ' . $copy['order']; } - /** - * Ensure GROUP BY posts.ID to deduplicate JOIN-multiplied rows. - * - * @param string|null $groupby Current GROUP BY SQL. - * @param \WP_Query $query Current WP_Query. - * @return string Modified GROUP BY SQL. - */ - public function posts_groupby( ?string $groupby, \WP_Query $query ): string { - $groupby = (string) ( $groupby ?? '' ); - $routed = $query->get( self::QUERY_VAR ); - if ( empty( $routed ) || ! is_array( $routed ) || ! empty( $query->get( self::COPY_VAR ) ) ) { - return $groupby; - } - - global $wpdb; - if ( '' === trim( $groupby ) ) { - $groupby = "`{$wpdb->posts}`.`ID`"; - } - - return $groupby; - } - /** * Drop SQL_CALC_FOUND_ROWS from a routed search; keep its COUNT for later. * @@ -365,7 +341,8 @@ class TMDO_Post_Query_Router { } $post_type = (string) array_key_first( $routed ); $groups = array_values( array_unique( array_column( $routed[ $post_type ], 'group' ) ) ); - if ( 1 !== count( $groups ) || ! in_array( $query->get( 'orderby' ), array( '', 'date', 'post_date' ), true ) ) { + if ( 1 !== count( $groups ) || ! in_array( $query->get( 'orderby' ), array( '', 'date', 'post_date' ), true ) + || in_array( 'NOT EXISTS', array_column( $routed[ $post_type ], 'compare' ), true ) ) { return array(); } $statuses = $query->get( 'post_status' ); diff --git a/tests/integration/PostQueryRouterCopyTest.php b/tests/integration/PostQueryRouterCopyTest.php index 823d0ff..2ce525a 100644 --- a/tests/integration/PostQueryRouterCopyTest.php +++ b/tests/integration/PostQueryRouterCopyTest.php @@ -58,7 +58,7 @@ class PostQueryRouterCopyTest extends TestCase { $mode->setValue( null, null ); } - /** @return array{join:string, where:string, orderby:string, groupby:string} */ + /** @return array{join:string, where:string, orderby:string} */ private function sql( array $vars ): array { $query = new WP_Query(); foreach ( array_merge( @@ -77,7 +77,6 @@ class PostQueryRouterCopyTest extends TestCase { 'join' => $router->posts_join( '', $query ), 'where' => $router->posts_where( " AND wp_posts.post_status = 'publish'", $query ), 'orderby' => $router->posts_orderby( 'wp_posts.post_date DESC', $query ), - 'groupby' => $router->posts_groupby( '', $query ), ); } @@ -85,7 +84,6 @@ class PostQueryRouterCopyTest extends TestCase { $this->assertStringContainsString( 'LEFT JOIN', $sql['join'] ); $this->assertStringNotContainsString( self::ALIAS . '.`post_status`', $sql['where'] ); $this->assertSame( 'wp_posts.post_date DESC', $sql['orderby'] ); - $this->assertSame( '`wp_itest_posts`.`ID`', $sql['groupby'] ); } public function test_ready_copies_filter_status_and_order_by_date_on_the_flat_table(): void { @@ -95,7 +93,6 @@ class PostQueryRouterCopyTest extends TestCase { $this->assertStringContainsString( self::ALIAS . ".`post_status` IN ('publish')", $sql['where'] ); $this->assertStringContainsString( "wp_posts.post_status = 'publish'", $sql['where'], 'WordPress status clause stays' ); $this->assertSame( self::ALIAS . '.`post_date` ASC', $sql['orderby'] ); - $this->assertSame( '', $sql['groupby'], 'one row per post: no GROUP BY, so LIMIT can stop early' ); } public function test_several_explicit_statuses_are_kept(): void { @@ -123,4 +120,9 @@ class PostQueryRouterCopyTest extends TestCase { $this->assert_legacy_shape( $this->sql( array( 'post_status' => '' ) ) ); $this->assert_legacy_shape( $this->sql( array( 'post_status' => 'any' ) ) ); } + + public function test_not_exists_keeps_the_legacy_shape(): void { + // The INNER JOIN would drop posts without a flat row, which NOT EXISTS must match. + $this->assert_legacy_shape( $this->sql( array( 'meta_query' => array( array( 'key' => 'rc_price', 'compare' => 'NOT EXISTS' ) ) ) ) ); + } } diff --git a/tests/integration/PostQueryRouterTest.php b/tests/integration/PostQueryRouterTest.php index 163e039..ecb3bd8 100644 --- a/tests/integration/PostQueryRouterTest.php +++ b/tests/integration/PostQueryRouterTest.php @@ -214,6 +214,34 @@ class PostQueryRouterTest extends TestCase { $this->assertStringContainsString( 'wpdo_post_wc_product', $join ); } + public function test_posts_join_joins_every_routed_group(): void { + $this->set_post_mode( 'aeav_only' ); + + $router = new WPDO_Post_Query_Router(); + $query = $this->make_query( array( + 'post_type' => 'product', + 'meta_query' => array( + array( 'key' => '_price', 'value' => '50', 'compare' => '>=', 'type' => 'NUMERIC' ), + array( 'key' => '_thumbnail_id', 'compare' => 'EXISTS' ), + ), + ) ); + + $router->pre_get_posts( $query ); + $join = $router->posts_join( '', $query ); + $where = $router->posts_where( '', $query ); + + $this->assertStringContainsString( 'wpdo_post_wc_product', $join ); + $this->assertStringContainsString( 'wpdo_post_wp_core', $join ); + $this->assertStringContainsString( '`wpdoflat_product_wp_core`.`_thumbnail_id` IS NOT NULL', $where ); + } + + public function test_no_group_by_is_added_for_one_row_per_post_joins(): void { + unset( $GLOBALS['_wp_filter_callbacks']['posts_groupby'] ); + ( new WPDO_Post_Query_Router() )->register_hooks(); + + $this->assertEmpty( $GLOBALS['_wp_filter_callbacks']['posts_groupby'] ?? array() ); + } + public function test_posts_where_appends_condition_for_routed_clause(): void { $this->set_post_mode( 'aeav_only' ); diff --git a/tests/live/README.md b/tests/live/README.md index 8b8d1f8..b35e0d5 100644 --- a/tests/live/README.md +++ b/tests/live/README.md @@ -39,7 +39,7 @@ Exit code 0 when every test passed, 1 otherwise. Output per test: | `shadow_write` | writes in shadow_read reach native meta (user, term, HivePress listing) | | `serialized` | array meta values are stored serialized in flat tables, never "Array" | | `resync` | dual_write / shadow_read: flat follows native after every write (4 entities × 2 modes: update, same value, delete, refused writes, stale flat, key rename by meta_id) | -| `router` | a routed WP_Query returns the same ids, in order, as native (OR, several post types, untyped numeric compares, named orderby, LIKE) | +| `router` | a routed WP_Query returns the same ids, in order, as native (OR, several post types, untyped numeric compares, named orderby, LIKE, keys of two groups, NOT EXISTS with the status/date copy route); routed SQL has no GROUP BY | | `zero_side_effect` | zone tables that were never created are never queried on save / delete | ## Tools (`tools/`) diff --git a/tests/live/suites/router.php b/tests/live/suites/router.php index cf63536..49d9c4c 100644 --- a/tests/live/suites/router.php +++ b/tests/live/suites/router.php @@ -6,7 +6,9 @@ * out of any meta_query and ANDed them, so OR relations (Q04, Q16), queries on * several post types (Q08), untyped comparisons on numeric columns (Q14, * native compares strings) and orderby on a named clause (Q15) returned other - * posts than WordPress would; LIKE compared without wildcards (Q06). + * posts than WordPress would; LIKE compared without wildcards (Q06). A key + * of a second group (wp_core _thumbnail_id, Q05) was never joined, and every + * routed query carried a GROUP BY although flat rows are one per post. * * Every case runs the same query twice on fixture posts — router hooks off, * then on with post mode shadow_read — and compares the ids in order. @@ -22,7 +24,7 @@ declare(strict_types=1); * @param array $args Query args (meta_query, post_type, orderby …). * @param int[] $ids Fixture posts to search within. * @param bool $routed Hook a TMDO_Post_Query_Router for this query. - * @return array{ids:int[], routed:bool} + * @return array{ids:int[], routed:bool, sql:string} */ function tmdo_live_router_query( array $args, array $ids, bool $routed ): array { global $wpdb; @@ -46,7 +48,7 @@ function tmdo_live_router_query( array $args, array $ids, bool $routed ): array ) ); } finally { - foreach ( array( 'pre_get_posts', 'posts_join', 'posts_where', 'posts_groupby', 'posts_orderby', 'posts_request', 'found_posts_query' ) as $hook ) { + foreach ( array( 'pre_get_posts', 'posts_join', 'posts_where', 'posts_orderby', 'posts_request', 'found_posts_query' ) as $hook ) { $priority = has_filter( $hook, array( $router, $hook ) ); if ( false !== $priority ) { remove_filter( $hook, array( $router, $hook ), $priority ); @@ -57,6 +59,7 @@ function tmdo_live_router_query( array $args, array $ids, bool $routed ): array return array( 'ids' => array_map( 'intval', $q->posts ), 'routed' => ! empty( $q->get( 'wpdo_post_clauses' ) ), + 'sql' => (string) $q->request, ); } @@ -107,14 +110,22 @@ function tmdo_live_router_listings(): array { * @param array $args Query args. * @param int[] $ids Fixture ids. * @param bool|null $expect_routed true / false to assert, null to ignore. + * @param bool $ordered Compare order too (false when the order has ties). */ -function tmdo_live_router_assert_native( array $args, array $ids, ?bool $expect_routed = null ): void { +function tmdo_live_router_assert_native( array $args, array $ids, ?bool $expect_routed = null, bool $ordered = true ): void { $native = tmdo_live_router_query( $args, $ids, false ); $routed = tmdo_live_router_query( $args, $ids, true ); - tmdo_live_assert_same( $native['ids'], $routed['ids'], 'routed ids (in order) vs native' ); + if ( ! $ordered ) { + sort( $native['ids'] ); + sort( $routed['ids'] ); + } + tmdo_live_assert_same( $native['ids'], $routed['ids'], 'routed ids vs native' ); if ( null !== $expect_routed ) { tmdo_live_assert_same( $expect_routed, $routed['routed'], 'query routed to the flat table' ); } + if ( $routed['routed'] ) { + tmdo_live_assert_true( false === stripos( $routed['sql'], 'GROUP BY' ), 'routed query adds no GROUP BY: ' . $routed['sql'] ); + } } TMDO_Live::test( @@ -246,3 +257,43 @@ TMDO_Live::test( } } ); + +TMDO_Live::test( + 'Q05 keys of two groups are both joined, routed', + static function () { + $l = tmdo_live_router_listings(); + TMDO_Live_Fixtures::field( 'post', '_thumbnail_id', 'hp_listing' ); + update_post_meta( $l['L1'], '_thumbnail_id', '123' ); + update_post_meta( $l['L3'], '_thumbnail_id', '124' ); + tmdo_live_router_assert_native( + array( + 'post_type' => 'hp_listing', + 'meta_query' => array( + array( 'key' => 'hp_hourly_rate', 'value' => 42, 'compare' => '>=', 'type' => 'NUMERIC' ), + array( 'key' => '_thumbnail_id', 'compare' => 'EXISTS' ), + ), + ), + array_values( $l ), + true + ); + } +); + +TMDO_Live::test( + 'NOT EXISTS with explicit statuses and date order keeps posts without a flat row', + static function () { + $l = tmdo_live_router_listings(); + tmdo_live_router_assert_native( + array( + 'post_type' => 'hp_listing', + 'post_status' => array( 'draft' ), + 'orderby' => 'date', + 'order' => 'DESC', + 'meta_query' => array( array( 'key' => 'hp_featured', 'compare' => 'NOT EXISTS' ) ), + ), + array_values( $l ), + true, + false + ); + } +);