Skip to content

Full-text search support#770

Closed
adamziel wants to merge 2 commits intoWordPress:masterfrom
adamziel:try/fulltext-search
Closed

Full-text search support#770
adamziel wants to merge 2 commits intoWordPress:masterfrom
adamziel:try/fulltext-search

Conversation

@adamziel
Copy link
Copy Markdown
Contributor

This PR proposes core support for fulltext search with fallback to LIKE when fulltext search isn't available.

Sites that would like to keep using LIKE could disable the fulltext_search_enabled option:

Zrzut ekranu 2020-11-26 o 17 12 46

Trac ticket: https://core.trac.wordpress.org/ticket/51769


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

}

if ( $wp_current_db_version < 51769 ) {
upgrade_570();
Copy link
Copy Markdown
Contributor Author

@adamziel adamziel Nov 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this form, it's a one-time only upgrade. Should we make it possible to retry at a later time (e.g. after migrating the site to more recent MySQL version)? If yes, what would be the best approach to do that?

if ( wp_fulltext_search_enabled() ) {
$search_orderby .= $wpdb->prepare( "WHEN MATCH({$wpdb->posts}.post_title) AGAINST (%s IN BOOLEAN MODE) THEN 1 ", $q['s'] );
} else {
$search_orderby .= $wpdb->prepare( "WHEN {$wpdb->posts}.post_title LIKE %s THEN 1 ", $like );
Copy link
Copy Markdown
Contributor Author

@adamziel adamziel Nov 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mimics LIKE behavior 1:1 for now. I believe MATCH() relevancy score would be a good fit here, maybe it could even replace WHEN/THEN entirely.

@adamziel adamziel changed the title Optional full-text search support Full-text search support Nov 26, 2020
upgrade_560();
}

if ( $wp_current_db_version < 51769 ) {
Copy link
Copy Markdown
Contributor Author

@adamziel adamziel Nov 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose this number arbitrarily (fulltext ticket id). We should discuss it more before merging.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's usually the SVN commit ID but your meaning is clear :) at time of writing it is 49632.

Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a couple of notes to the PR and will catch up on the discussion on the ticket.

upgrade_560();
}

if ( $wp_current_db_version < 51769 ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's usually the SVN commit ID but your meaning is clear :) at time of writing it is 49632.

* @ignore
* @since 5.7.0
*/
function upgrade_570() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excluding the option I think the rest can be done by updating schema.php

Comment on lines +211 to +220
<?php if ( wp_fulltext_search_available() ) : ?>
<tr class="option-site-visibility">
<th scope="row"><?php _e( 'Full-text search' ); ?> </th>
<td><fieldset><legend class="screen-reader-text"><span><?php _e( 'Full-text search' ); ?> </span></legend>
<label for="fulltext_search_enabled"><input name="fulltext_search_enabled" type="checkbox" id="fulltext_search_enabled" value="1" <?php echo checked( '1', get_option( 'fulltext_search_enabled' ) ); ?> />
<?php _e( 'Use full-text search on this site' ); ?></label>
</fieldset></td>
</tr>
<?php endif ?>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference would be to go without an option. If the indexes are always created then I am not sure what the gain is in giving admins options they may not understand.

Is there a common reason an owner might want to turn it off if it's available on their server?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah +1 to not making this a UI option.

*/
function wp_fulltext_search_available() {
$is_available = get_option( 'fulltext_search_available' ) === '1';
return apply_filters( 'fulltext_search_available', $is_available );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔢 Docblock on filters (but can wait as the ticket is discussed, tbh)

/*
* Create fulltext indexes for `wp_posts`.
*/
$result = $wpdb->query( "ALTER TABLE $wpdb->posts ADD FULLTEXT INDEX `wp_posts_fulltext` (`post_title` ASC, `post_excerpt` ASC, `post_content` ASC);" );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nicer way of detecting this that can be added to the $wpdb->has_cap() method would be dandy.

@adamziel
Copy link
Copy Markdown
Contributor Author

I'm not pursuing this work anymore so I'm closing this PR. Feel free to take over!

@adamziel adamziel closed this Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants