's' term in WP_Query extended class causes huge memory leak (268MB)

Question

I've used WP-Geo-Posts as a starting point for building a plugin which extends WooCommerce to support classifieds-like functionality. Using ACF I've associated products with a location (latitude and longitude) and I'm now building location support into search. Users provide search terms, a location and a radius. Results should be products within a certain radius, ordered by closest to the provided location. It's working perfectly, but the search is currently ignoring any search terms the user enters.

I've made a class which extends WP_Query to build the pseudo-field of 'distance'. You'd think I could just provide s to the same $args array, and it would scope by keyword like WP_Query does. Instead I get a blank page and the logs say

 PHP Fatal error:  Allowed memory size of 268435456 bytes exhausted (tried to allocate 130968 bytes) in /home/mydir/home/mysite/public_html/wp-includes/query.php on line 1681, referer: http://example.com/shop/

I have this which works fine:

$args = array(
  'post_type'       => $query->get('post_type') ,
  'posts_per_page'  => 20,
  'fields'          => 'all',
  'lat'             => $_REQUEST['lat'],
  'lng'             => $_REQUEST['lng'],
  'distance'        => $_REQUEST['dist']
);
$query = new WP_Query_Geo( $args );

And this which leaks memory like gangbusters:

$args = array(
  'post_type'       => $query->get('post_type') ,
  'posts_per_page'  => 20,
  'fields'          => 'all',
  'lat'             => $_REQUEST['lat'],
  'lng'             => $_REQUEST['lng'],
  'distance'        => $_REQUEST['dist'],
  's' => $search_terms
);
$query = new WP_Query_Geo( $args );

My class which extends WP_Query (WP_Query_Geo) uses concats (.=) on $fields, $join and $where so I don't see why providing s should do any harm.

This is the output of var_dump($query->request); without the s in $args:

string(799) "SELECT SQL_CALC_FOUND_ROWS wp_posts.*, ( 6371 * acos( cos( radians(48.4284207) ) * cos( radians( latitude.meta_value ) ) * cos( radians( longitude.meta_value ) - radians(-123.36564440000001) ) + sin( radians(48.4284207) ) * sin( radians( latitude.meta_value ) ) ) ) AS distance , latitude.meta_value AS latitude , longitude.meta_value AS longitude FROM wp_posts INNER JOIN wp_postmeta AS latitude ON wp_posts.ID = latitude.post_id INNER JOIN wp_postmeta AS longitude ON wp_posts.ID = longitude.post_id WHERE 1=1 AND wp_posts.post_type = 'product' AND (wp_posts.post_status = 'publish') AND latitude.meta_key="lat" AND longitude.meta_key="lng" HAVING distance <= 100 ORDER BY distance ASC, wp_posts.post_date DESC LIMIT 0, 20"

Answer 1

I'm not sure what convention is with pre_get_posts, but it seems to be for setting additional arguments and not replacing then running the query. Comparing wp_query and my own geo query, I noticed my own had post data in the query object. This may have been what caused the memory leak, but it must have had an infinite loop - my entire DB is no where near 268MB. Perhaps the query is not meant to take place inside the constructor? I'd love to know of a solution using pre_get_posts.

Instead of using pre_get_posts, I used the woocommerce_before_shop_loop hook to replace $wp_query altogether.

add_action('woocommerce_before_shop_loop', 'wr_product_geo_query');
function wr_product_geo_query() {
  global $wp_query;

  if ($wp_query->is_search() && isset($_REQUEST['loc'])) {
    $args = array(
      'post_type'       => $_REQUEST['post_type'] ,
      'posts_per_page'  => 20,
      'fields'          => 'all',
      'lat'             => $_REQUEST['lat'],
      'lng'             => $_REQUEST['lng'],
      'distance'        => $_REQUEST['dist'],
      's' => $_REQUEST['s']
    );

    $wp_query = new WP_Query_Geo( $args );
  }
}

Pretty simple, but I'd like a solution that inserts itself into default search behaviour instead of relying on a template hook.

Update: After much internal deliberation, I believe pre_get_posts is exclusively for modifying an existing query. I'm not modifying it, I'm nuking it. In which case, creating a new query object via a custom template or template hook is the "proper" way to do it.