Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add include, lower_limit_object_id and higher_limit_object_id for com… #4074

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
+139 −42
Draft

Add include, lower_limit_object_id and higher_limit_object_id for com… #4074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 100 additions & 24 deletions includes/classes/Indexable/Comment/Comment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,22 +777,30 @@ public function get_indexable_comment_status() {
* @return array
*/
public function query_db( $args ) {

$defaults = [
'type' => $this->get_indexable_comment_types(),
'status' => $this->get_indexable_comment_status(),
'post_type' => Indexables::factory()->get( 'post' )->get_indexable_post_types(),
'post_status' => Indexables::factory()->get( 'post' )->get_indexable_post_status(),
'number' => $this->get_bulk_items_per_page(),
'offset' => 0,
'orderby' => 'comment_ID',
'order' => 'desc',
'type' => $this->get_indexable_comment_types(),
'status' => $this->get_indexable_comment_status(),
'post_type' => Indexables::factory()->get( 'post' )->get_indexable_post_types(),
'post_status' => Indexables::factory()->get( 'post' )->get_indexable_post_status(),
'number' => $this->get_bulk_items_per_page(),
'offset' => 0,
'orderby' => 'comment_ID',
'order' => 'desc',
'ep_indexing_advanced_pagination' => true,
];

if ( isset( $args['per_page'] ) ) {
$args['number'] = $args['per_page'];
}

if ( isset( $args['include'] ) ) {
$args['comment__in'] = $args['include'];
}

if ( isset( $args['exclude'] ) ) {
$args['comment__not_in'] = $args['exclude'];
}

/**
* Filter database arguments for comment query
*
Expand All @@ -809,23 +817,34 @@ public function query_db( $args ) {
unset( $all_query_args['offset'] );
$all_query_args['count'] = true;

/**
* Filter database arguments for comment count query
*
* @hook ep_comment_all_query_db_args
* @param {array} $args Query arguments based to WP_Comment_Query
* @since 3.6.0
* @return {array} New arguments
*/
$total_objects = get_comments( apply_filters( 'ep_comment_all_query_db_args', $all_query_args, $args ) );

if ( ! empty( $args['offset'] ) ) {
if ( (int) $args['offset'] >= $total_objects ) {
$total_objects = 0;
}
if ( isset( $args['comment__in'] ) || 0 < $args['offset'] ) {
// Disable advanced pagination. Not useful if only indexing specific IDs.
$args['ep_indexing_advanced_pagination'] = false;
}

$query = new WP_Comment_Query( $args );
// Enforce the following query args during advanced pagination to ensure things work correctly.
if ( $args['ep_indexing_advanced_pagination'] ) {
$args = array_merge(
$args,
[
'suppress_filters' => false,
'orderby' => 'ID',
'order' => 'DESC',
'paged' => 1,
'offset' => 0,
'no_found_rows' => false,
]
);
add_filter( 'comments_clauses', array( $this, 'bulk_indexing_filter_comments_where' ), 9999, 2 );

$query = new WP_Comment_Query( $args );
$total_objects = $query->found_comments;

remove_filter( 'comments_clauses', array( $this, 'bulk_indexing_filter_comments_where' ), 9999, 2 );
} else {
$query = new WP_Comment_Query( $args );
$total_objects = $query->found_comments;
}

if ( is_array( $query->comments ) ) {
array_walk( $query->comments, [ $this, 'remap_comments' ] );
Expand All @@ -837,6 +856,50 @@ public function query_db( $args ) {
];
}

/**
* Manipulate the WHERE clause of the bulk indexing query to paginate by ID in order to avoid performance issues with SQL offset.
*
* @param array $clauses Array of clauses for the query.
* @param WP_Query $query \WP_Comment_Query object.
* @return array Updated array of clauses.
*/
public function bulk_indexing_filter_comments_where( $clauses, $query ) {
global $wpdb;

$using_advanced_pagination = $this->get_query_var( $query, 'ep_indexing_advanced_pagination', false );

if ( $using_advanced_pagination ) {
$requested_upper_limit_id = $this->get_query_var( $query, 'ep_indexing_upper_limit_object_id', PHP_INT_MAX );
$requested_lower_limit_post_id = $this->get_query_var( $query, 'ep_indexing_lower_limit_object_id', 0 );
$last_processed_id = $this->get_query_var( $query, 'ep_indexing_last_processed_object_id', null );

// On the first loopthrough we begin with the requested upper limit ID. Afterwards, use the last processed ID to paginate.
$upper_limit_range_post_id = $requested_upper_limit_id;
if ( is_numeric( $last_processed_id ) ) {
$upper_limit_range_post_id = $last_processed_id - 1;
}

// Sanitize. Abort if unexpected data at this point.
if ( ! is_numeric( $upper_limit_range_post_id ) || ! is_numeric( $requested_lower_limit_post_id ) ) {
return $clauses;
}

$range = [
'upper_limit' => "{$wpdb->comments}.comment_ID <= {$upper_limit_range_post_id}",
'lower_limit' => "{$wpdb->comments}.comment_ID >= {$requested_lower_limit_post_id}",
];

// Skip the end range if it's unnecessary.
$skip_ending_range = 0 === $requested_lower_limit_post_id;
$where = $clauses['where'];
$where = $skip_ending_range ? " {$range['upper_limit']} AND {$where}" : " {$range['upper_limit']} AND {$range['lower_limit']} AND {$where}";

$clauses['where'] = $where;
}

return $clauses;
}

/**
* Prepare a comment document for indexing
*
Expand Down Expand Up @@ -1073,4 +1136,17 @@ protected function parse_orderby( $orderby, $order, $args ) {

return $sort;
}

/**
* Retrieve a specific query variable from the query object.
*
* @param \WP_Comment_Query $query The query object.
* @param string $query_var The name of the query variable to retrieve.
* @param string $default_value The default value to return if the query variable is not set. Default is an empty string.
*
* @return mixed The value of the query variable if set, otherwise the default value.
*/
public function get_query_var( $query, $query_var, $default_value = '' ) {
return $query->query_vars[ $query_var ] ?? $default_value;
}
}
20 changes: 11 additions & 9 deletions includes/classes/Indexable/Post/Post.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,16 @@ public function query_db( $args ) {
];
}

/**
* Manipulate the WHERE clause of the bulk indexing query to paginate by ID in order to avoid performance issues with SQL offset.
*
* @param string $where The current $where clause.
* @param WP_Query $query WP_Query object.
* @return string WHERE clause with our pagination added if needed.
*/
/**
* Manipulate the WHERE clause of the bulk indexing query to paginate by ID in order to avoid performance issues with SQL offset.
*
* @param string $where The current $where clause.
* @param WP_Query $query WP_Query object.
* @return string WHERE clause with our pagination added if needed.
*/
public function bulk_indexing_filter_posts_where( $where, $query ) {
global $wpdb;

$using_advanced_pagination = $query->get( 'ep_indexing_advanced_pagination', false );

if ( $using_advanced_pagination ) {
Expand All @@ -161,8 +163,8 @@ public function bulk_indexing_filter_posts_where( $where, $query ) {
}

$range = [
'upper_limit' => "{$GLOBALS['wpdb']->posts}.ID <= {$upper_limit_range_post_id}",
'lower_limit' => "{$GLOBALS['wpdb']->posts}.ID >= {$requested_lower_limit_post_id}",
'upper_limit' => "{$wpdb->posts}.ID <= {$upper_limit_range_post_id}",
'lower_limit' => "{$wpdb->posts}.ID >= {$requested_lower_limit_post_id}",
];

// Skip the end range if it's unnecessary.
Expand Down
37 changes: 28 additions & 9 deletions tests/php/indexables/TestComment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2179,25 +2179,33 @@ public function testWooCommerceReviewIndexing() {
* @group comment
*/
public function testCommentIndexableQueryDb() {
$post_id = wp_insert_post(
ElasticPress\Features::factory()->deactivate_feature( 'woocommerce' );

$post_id = $this->ep_factory->post->create();

$comment_1_id = $this->ep_factory->comment->create(
[
'post_name' => 'start-here',
'post_status' => 'publish',
'comment_post_ID' => $post_id,
]
);

wp_insert_comment(
$this->ep_factory->comment->create_many(
2,
[
'comment_content' => 'Test comment 1',
'comment_post_ID' => $post_id,
]
);

$product_id = wp_insert_post(
$product_id = $this->ep_factory->product->create(
[
'post_content' => 'product 1',
'post_type' => 'product',
'post_status' => 'publish',
]
);

$this->ep_factory->comment->create(
[
'comment_post_ID' => $product_id,
'comment_type' => 'review',
]
);

Expand All @@ -2211,12 +2219,23 @@ public function testCommentIndexableQueryDb() {

$comment_indexable = new \ElasticPress\Indexable\Comment\Comment();

// Test only comments are returned.
$results = $comment_indexable->query_db( [] );

$this->assertArrayHasKey( 'objects', $results );
$this->assertArrayHasKey( 'total_objects', $results );
$this->assertEquals( 3, $results['total_objects'] );

// Test only 1 comment is returned.
$results = $comment_indexable->query_db( [ 'include' => $comment_1_id ] );
$this->assertArrayHasKey( 'objects', $results );
$this->assertArrayHasKey( 'total_objects', $results );
$this->assertEquals( 1, $results['total_objects'] );

// Test all comments are returned except the one with ID.
$results = $comment_indexable->query_db( [ 'exclude' => $comment_1_id ] );
$this->assertArrayHasKey( 'objects', $results );
$this->assertArrayHasKey( 'total_objects', $results );
$this->assertEquals( 2, $results['total_objects'] );
}

/**
Expand Down
Loading