Query: key the result cache on what the query-clauses filter changed.

CHANGELOG.md

@@ -4,6 +4,12 @@ Notable changes to BerlinDB are documented here.
 
 ## 3.1.0 - Unreleased
 
+- Keys the result cache on what a `{prefix}_{items}_query_clauses` filter changed
+  (#275). A callback that narrows rows on a query var with no registered default, or
+  on state outside the query vars, no longer shares one cache entry with the
+  unfiltered query. The request is now built before the cache lookup, so the filter
+  runs on cache hits too; a query no callback changes keeps its previous key.
+
 - Makes `MetaStore` the ownership boundary for item-meta cleanup (#262). A
   BerlinDB-owned Meta preset now derives its meta-row primary column and owning-object
   column from its registered Schema, so customized names are deleted through the

CLAUDE.md

@@ -46,7 +46,7 @@ hand-rolled queries — never more surprising.
    vendor/bin/phpcs
    ```
 5. **Don't invent APIs.** If unsure how something behaves, search `src/` and
-   `tests/` — the source and its 1755 test methods are the source of truth, ahead
+   `tests/` — the source and its 1760 test methods are the source of truth, ahead
    of memory or training data. (PHPUnit reports more cases: data providers expand
    methods at run time.)
 6. **Keep changes focused and tested.** Bug fixes and new behavior ship with

src/Database/Traits/Query/Aggregates.php

@@ -270,30 +270,24 @@ private function empty_aggregate_result( array $aggregate ): array {
 	}
 
 	/**
-	 * Run the 'aggregate' container and return its results.
+	 * Build the request SQL for the 'aggregate' container.
 	 *
-	 * Called from the query path (get_item_ids()) once the query vars are parsed, so
-	 * it reads the compiled WHERE/JOIN directly. Each canonical entry renders
+	 * Called from the query path (set_item_ids_request()) once the query vars are
+	 * parsed, so it reads the compiled WHERE/JOIN directly. Each canonical entry renders
 	 * "FUNC( column ) AS alias". A JOIN-producing filter is wrapped in a
 	 * distinct-primary subquery (the same dedup the scalar methods use), so a
-	 * one-to-many join does not fan out and double-count.
-	 *
-	 * Without a 'groupby' var the whole set is ONE row: an assoc keyed by every
-	 * requested alias, each value null when its aggregate is NULL (an empty set) -
-	 * unlike COUNT, an empty SUM/MAX is null, not zero. With 'groupby' the group
-	 * column(s) join the SELECT and drive a GROUP BY (grouped AFTER the fan-out dedup),
-	 * so it returns a LIST of rows, each carrying the group column(s) plus each alias;
-	 * an empty set is an empty list (no groups).
+	 * one-to-many join does not fan out and double-count. With a 'groupby' var the
+	 * group column(s) join the SELECT and drive a GROUP BY (grouped AFTER the fan-out
+	 * dedup).
 	 *
 	 * @since 3.1.0
 	 *
 	 * @param array<string,array{function: string, column: string}> $aggregate The
 	 *        canonical aggregate container (from normalize_aggregate_container()).
 	 *
-	 * @return array<string,string|null>|array<int,array<string,mixed>> One row keyed by
-	 *         alias, or a list of grouped rows.
+	 * @return string The request SQL, or '' when no entry renders.
 	 */
-	private function run_aggregate( array $aggregate ): array {
+	private function get_aggregate_request( array $aggregate ): string {
 
 		/*
 		 * A 'groupby' var puts the group column(s) into the SELECT and drives a GROUP BY,
@@ -336,7 +330,7 @@ private function run_aggregate( array $aggregate ): array {
 
 		// Nothing renderable resolves to an empty result set.
 		if ( empty( $fields ) ) {
-			return array();
+			return '';
 		}
 
 		$group_clause = ( true === $grouped )
@@ -381,7 +375,36 @@ private function run_aggregate( array $aggregate ): array {
 			$clauses[ 'orderby' ] = $this->resolve_aggregate_orderby( $group_names, array_keys( $fields ) );
 		}
 
-		$request = $this->parse_request_clauses( $clauses );
+		return $this->parse_request_clauses( $clauses );
+	}
+
+	/**
+	 * Run the 'aggregate' container's request and return its results.
+	 *
+	 * Runs the request get_aggregate_request() built. Without a 'groupby' var the
+	 * whole set is ONE row: an assoc keyed by every requested alias, each value null
+	 * when its aggregate is NULL (an empty set) - unlike COUNT, an empty SUM/MAX is
+	 * null, not zero. With 'groupby' it returns a LIST of rows, each carrying the group
+	 * column(s) plus each alias; an empty set is an empty list (no groups).
+	 *
+	 * @since 3.1.0
+	 *
+	 * @param array<string,array{function: string, column: string}> $aggregate The
+	 *        canonical aggregate container (from normalize_aggregate_container()).
+	 * @param string                                                $request   The
+	 *        request SQL from get_aggregate_request().
+	 *
+	 * @return array<string,string|null>|array<int,array<string,mixed>> One row keyed by
+	 *         alias, or a list of grouped rows.
+	 */
+	private function run_aggregate( array $aggregate, string $request ): array {
+
+		// Nothing renderable resolves to an empty result set.
+		if ( '' === $request ) {
+			return array();
+		}
+
+		$grouped = ! empty( $this->get_valid_groupby_columns( (array) $this->get_query_var( 'groupby' ) ) );
 
 		// Grouped: a list of rows (group column(s) + aliases), as the database returns them.
 		if ( true === $grouped ) {

src/Database/Traits/Query/Cache.php

@@ -60,18 +60,23 @@ abstract protected function get_results_invariant_vars(): array;
 	abstract protected function get_items_raw( string $where ): array;
 
 	/**
-	 * Get cache key from $query_vars and $query_var_defaults.
+	 * Get cache key from $query_vars, $query_var_defaults, and filtered clauses.
 	 *
 	 * Performs the following operations to create a consistent cache-key:
 	 * - Removes the "fields" query_var, because whole objects/items are cached
 	 * - Removes unknown or unregistered query_var keys
 	 * - Sorts query_vars by query_var_default keys
 	 * - Removes query_vars with default values
-	 * - Serializes and md5 hashes query_vars
+	 * - Adds the clauses the query-clauses filter changed, if any
+	 * - Serializes and md5 hashes the result
 	 * - Combines plural name and hash independently of the cache generation
 	 *
+	 * The request must be built first (set_item_ids_request()), so the
+	 * query-clauses filter has run.
+	 *
 	 * @since 1.0.0
 	 * @since 2.1.0 Correctly removes unique query_var_default_value values
+	 * @since 3.1.0 Includes the clauses the query-clauses filter changed
 	 *
 	 * @return string
 	 */
@@ -102,13 +107,52 @@ private function get_cache_key(): string {
 			$slice[ $key ] = $this->query_vars[ $key ];
 		}
 
+		/*
+		 * A query-clauses callback can narrow the rows on a var with no registered
+		 * default, or on state outside the query vars entirely, so neither reaches
+		 * the slice. Key on what the filter changed; an unfiltered key is unchanged.
+		 */
+		$filtered = $this->get_current_array( 'filtered_clauses' );
+		$hashable = empty( $filtered )
+			? $slice
+			: array( $slice, $filtered );
+
 		// Hash the sliced query vars. serialize() is intentional and safe here.
-		$hash = md5( serialize( $slice ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize
+		$hash = md5( serialize( $hashable ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize
 
 		// Return the namespaced cache key.
 		return "get_{$this->get_item_name_plural()}:{$hash}";
 	}
 
+	/**
+	 * Record which query clauses the query-clauses filter changed.
+	 *
+	 * A removed clause is recorded as null. get_cache_key() hashes the result.
+	 *
+	 * @since 3.1.0
+	 *
+	 * @param array<string,mixed> $clauses  The clauses before the filter.
+	 * @param array<string,mixed> $filtered The clauses after the filter.
+	 * @return void
+	 */
+	private function set_filtered_clauses( array $clauses, array $filtered ): void {
+		$changed = array();
+
+		foreach ( $filtered as $key => $value ) {
+			if ( ! array_key_exists( $key, $clauses ) || ( $clauses[ $key ] !== $value ) ) {
+				$changed[ $key ] = $value;
+			}
+		}
+
+		foreach ( array_keys( array_diff_key( $clauses, $filtered ) ) as $key ) {
+			$changed[ $key ] = null;
+		}
+
+		ksort( $changed );
+
+		$this->set_current( 'filtered_clauses', $changed );
+	}
+
 	/**
 	 * Get the current cache generations for tables used by relationship filters.
 	 *

src/Database/Traits/Query/Execution.php

@@ -149,6 +149,9 @@ private function get_items(): array|int {
 			return $this->items;
 		}
 
+		// Build the request first, so the cache key sees what the query-clauses filter changed.
+		$this->set_item_ids_request();
+
 		/*
 		 * Check the cache. EXPLAIN returns a plan (not rows) and must reflect the
 		 * current optimizer state, so it is never served from or stored in the cache.
@@ -288,30 +291,46 @@ private function set_request(): void {
 	}
 
 	/**
-	 * Used internally to get a list of item IDs matching the query vars.
+	 * Build and store the request SQL for the resolved query mode.
 	 *
-	 * @since 1.0.0
-	 * @since 3.0.0 Uses wp_parse_list() instead of wp_parse_id_list()
+	 * Runs before the result-cache lookup, because get_cache_key() includes the
+	 * clauses the query-clauses filter changed.
 	 *
-	 * @return array<bool|float|int|string>|array<string,mixed>[]|array<string,string|null>|int Item IDs for a full query, int/rows for a count query, or aggregate results keyed by alias.
+	 * @since 3.1.0
 	 */
-	private function get_item_ids(): array|int {
+	private function set_item_ids_request(): void {
 
-		/*
-		 * Aggregate mode computes its result from its own request (run_aggregate()),
-		 * with no primary-ID selection, so it returns before the shared request below.
-		 */
+		// Aggregate mode builds its own request, with no primary-ID selection.
 		if ( 'aggregate' === $this->get_query_mode() ) {
-			return $this->run_aggregate( (array) $this->get_query_var( 'aggregate' ) );
+			$this->set_current( 'request', $this->get_aggregate_request( (array) $this->get_query_var( 'aggregate' ) ) );
+			return;
 		}
 
-		// Count and rows both run the assembled request; build it once.
+		// Count and rows both run the assembled request.
 		$this->set_query_clauses();
 		$this->set_request_clauses();
 		$this->set_request();
+	}
 
+	/**
+	 * Used internally to get a list of item IDs matching the query vars.
+	 *
+	 * Runs the request set_item_ids_request() built.
+	 *
+	 * @since 1.0.0
+	 * @since 3.0.0 Uses wp_parse_list() instead of wp_parse_id_list()
+	 * @since 3.1.0 No longer builds the request.
+	 *
+	 * @return array<bool|float|int|string>|array<string,mixed>[]|array<string,string|null>|int Item IDs for a full query, int/rows for a count query, or aggregate results keyed by alias.
+	 */
+	private function get_item_ids(): array|int {
 		$request = $this->get_current_string( 'request' ) ?? '';
 
+		// Aggregate mode shapes its own result, with no primary-ID selection.
+		if ( 'aggregate' === $this->get_query_mode() ) {
+			return $this->run_aggregate( (array) $this->get_query_var( 'aggregate' ), $request );
+		}
+
 		// Dispatch to the mode's execution.
 		return ( 'count' === $this->get_query_mode() )
 			? $this->execute_count( $request )

src/Database/Traits/Query/Filters.php

@@ -173,6 +173,7 @@ protected function filter_found_items_query( $sql = '' ) {
 	 * Filter the query clauses before they are parsed into a SQL string.
 	 *
 	 * @since 3.0.0
+	 * @since 3.1.0 Records the clauses the filter changed, for the result-cache key.
 	 *
 	 * @param array<string,mixed> $clauses All of the SQL query clauses.
 	 * @return array<string,mixed>
@@ -183,6 +184,7 @@ protected function filter_query_clauses( $clauses = array() ) {
 		$filter_name = $this->apply_hook_prefix( $this->get_item_name_plural() . '_query_clauses' );
 
 		if ( '' === $filter_name ) {
+			$this->set_filtered_clauses( (array) $clauses, (array) $clauses );
 			return $clauses;
 		}
 
@@ -194,12 +196,16 @@ protected function filter_query_clauses( $clauses = array() ) {
 		 * @param array<string,mixed> $clauses An array of query clauses.
 		 * @param Query               $query   Current query instance.
 		 */
-		return (array) apply_filters_ref_array(
+		$filtered = (array) apply_filters_ref_array(
 			$filter_name,
 			array(
 				$clauses,
 				&$this,
 			)
 		);
+
+		$this->set_filtered_clauses( (array) $clauses, $filtered );
+
+		return $filtered;
 	}
 }

tests/Database/Kern/Query/CacheKeyGuardTest.php

@@ -228,6 +228,64 @@ public function test_results_affecting_vars_segment_cache_key() {
 		);
 	}
 
+	/**
+	 * Test the other direction: a var with no registered default that only a
+	 * query-clauses callback reads still segments the key, because the callback
+	 * changes which rows return.
+	 *
+	 * @since 3.1.0
+	 */
+	public function test_var_read_only_by_clauses_filter_segments_cache_key() {
+		$callback = static function ( $clauses, $query ) {
+			$name = (string) $query->get_query_var( 'only_name' );
+
+			if ( '' !== $name ) {
+				global $wpdb;
+				$clauses[ 'where' ] = $wpdb->prepare( 'WHERE name = %s', $name );
+			}
+
+			return $clauses;
+		};
+
+		add_filter( 'berlindb_database_widgets_query_clauses', $callback, 10, 2 );
+		try {
+			$plain    = $this->cache_key( array( 'number' => 10 ) );
+			$filtered = $this->cache_key(
+				array(
+					'number'    => 10,
+					'only_name' => 'beta',
+				)
+			);
+		} finally {
+			remove_filter( 'berlindb_database_widgets_query_clauses', $callback, 10 );
+		}
+
+		$this->assertNotSame( $plain, $filtered );
+	}
+
+	/**
+	 * Test that a query-clauses callback which changes nothing does not segment the
+	 * key: hooking the filter alone must not fragment every query's cache entry.
+	 *
+	 * @since 3.1.0
+	 */
+	public function test_clauses_filter_that_changes_nothing_does_not_segment_cache_key() {
+		$plain = $this->cache_key( array( 'number' => 10 ) );
+
+		$callback = static function ( $clauses ) {
+			return $clauses;
+		};
+
+		add_filter( 'berlindb_database_widgets_query_clauses', $callback );
+		try {
+			$hooked = $this->cache_key( array( 'number' => 10 ) );
+		} finally {
+			remove_filter( 'berlindb_database_widgets_query_clauses', $callback );
+		}
+
+		$this->assertSame( $plain, $hooked );
+	}
+
 	/**
 	 * Test that RESULTS_INVARIANT_VARS matches the reviewed list and contains only
 	 * real query vars (no typos, no drift).