Keep the FULLTEXT index on searches with short words or stopwords (#4970)

backend/handler/database/roms_handler.py

@@ -91,6 +91,11 @@ from utils.database import (
     release_day_ranges,
     rom_unset_flag_column,
 )
+from utils.fulltext import (
+    FULLTEXT_TOKEN_REGEX,
+    fulltext_settings,
+    split_fulltext_words,
+)
 from utils.platform_slugs import UniversalPlatformSlug as UPS
 from utils.sql_dialect import (
     Analyze,
@@ -102,7 +107,7 @@ from utils.sql_dialect import (
     nulls_last,
 )
 
-from .base_handler import DBBaseHandler, affected_rows
+from .base_handler import DBBaseHandler, affected_rows, sync_engine
 
 type RomSelect = Select[tuple[Rom]]
 
@@ -161,12 +166,6 @@ RUFFLE_SUPPORTED_PLATFORMS = [
     UPS.BROWSER,
 ]
 
-# Used to remove native full-text SQL operators
-FULLTEXT_BOOLEAN_OPERATORS_REGEX = re.compile(r'[+\-~<>()"@*]')
-
-# 3 is the default minimum size in InnoDB
-FULLTEXT_MIN_TOKEN_SIZE = 3
-
 # A term reaches the hash columns only when it is hex of exactly a digest
 # length, so an ordinary name search builds no hash SQL at all. Hashes are
 # stored lowercase, which keeps the lookup an indexed equality.
@@ -236,6 +235,12 @@ ROM_SEARCH_COLUMNS: tuple[QueryableAttribute[Any], ...] = tuple(
 )
 
 
+def _name_like(word: str) -> ColumnElement[bool]:
+    return or_(
+        *(column.icontains(word, autoescape=True) for column in ROM_SEARCH_COLUMNS)
+    )
+
+
 def _fulltext_match(boolean_query: str) -> ColumnElement[Any]:
     """A MariaDB/MySQL FULLTEXT match of the ROM's name, filename and aliases."""
     return fulltext_match(
@@ -246,13 +251,7 @@ def _fulltext_match(boolean_query: str) -> ColumnElement[Any]:
 
 def _search_relevance(phrases: Sequence[str]) -> ColumnElement[Any]:
     """How well the ROM's name, filename or aliases hold the phrases, words in order."""
-    holds_a_phrase = or_(
-        *(
-            column.icontains(phrase, autoescape=True)
-            for phrase in phrases
-            for column in ROM_SEARCH_COLUMNS
-        )
-    )
+    holds_a_phrase = or_(*(_name_like(phrase) for phrase in phrases))
     return DialectCase(
         postgresql=case((holds_a_phrase, 1), else_=0),
         mysql=_fulltext_match(" ".join(f'"{phrase}"' for phrase in phrases)),
@@ -1131,48 +1130,40 @@ class DBRomsHandler(DBBaseHandler):
             )
         )
 
-    def _build_fulltext_boolean_query(self, term: str) -> str | None:
-        words = FULLTEXT_BOOLEAN_OPERATORS_REGEX.sub(" ", term).split()
-        if not words or any(len(word) < FULLTEXT_MIN_TOKEN_SIZE for word in words):
-            return None
-        return " ".join(f"+{word}*" for word in words)
-
     def _build_search_phrases(self, search_term: str) -> list[str]:
         """The multi-word terms of a search, which relevance ranks by."""
         phrases: list[str] = []
         for term in search_term.split("|"):
-            words = FULLTEXT_BOOLEAN_OPERATORS_REGEX.sub(" ", term).split()
+            words = FULLTEXT_TOKEN_REGEX.findall(term)
             if len(words) > 1:
                 phrases.append(" ".join(words))
         return phrases
 
     def _build_name_conditions(self, terms: Sequence[str]) -> list[Any]:
         """One condition per term, matching it against the ROM's name, filename and aliases."""
-        # PostgreSQL's pg_trgm indexes serve the ILIKE; MariaDB and MySQL use
-        # their FULLTEXT index unless a word is too short for it.
-        like_conditions = [
-            and_(
-                *(
-                    or_(*(column.ilike(f"%{word}%") for column in ROM_SEARCH_COLUMNS))
-                    for word in term.split()
+        settings = fulltext_settings(sync_engine)
+        conditions: list[Any] = []
+        for term in terms:
+            words = term.split()
+            likes = {word: _name_like(word) for word in words}
+            like = and_(*likes.values())
+            # Unreadable settings leave LIKE alone to decide.
+            indexed, unindexed = (
+                split_fulltext_words(words, settings) if settings else ([], words)
+            )
+            if not indexed:
+                conditions.append(like)
+                continue
+            # PostgreSQL's pg_trgm indexes serve the ILIKE. On MariaDB and MySQL
+            # FULLTEXT drives the query and LIKE checks the words it can't hold.
+            match = _fulltext_match(" ".join(f"+{word}*" for word in indexed))
+            conditions.append(
+                DialectCase(
+                    postgresql=like,
+                    mysql=and_(match, *(likes[word] for word in unindexed)),
                 )
             )
-            for term in terms
-        ]
-        boolean_queries = [
-            query
-            for term in terms
-            if (query := self._build_fulltext_boolean_query(term)) is not None
-        ]
-        if len(boolean_queries) < len(terms):
-            return like_conditions
-
-        return [
-            DialectCase(postgresql=like, mysql=_fulltext_match(boolean_query))
-            for boolean_query, like in zip(
-                boolean_queries, like_conditions, strict=True
-            )
-        ]
+        return conditions
 
     def _build_hash_selects(self, terms: Iterable[str]) -> list[Select[tuple[int]]]:
         """Id-yielding selects for terms shaped like a hash digest.

backend/tests/handler/database/test_roms_fulltext_search.py

@@ -0,0 +1,39 @@
+from unittest.mock import patch
+
+import pytest
+from sqlalchemy import select
+from sqlalchemy.engine import Dialect
+from tests.sql_dialects import MARIADB_DIALECT, POSTGRESQL_DIALECT, compile_sql
+
+from handler.database import db_rom_handler, roms_handler
+from models.rom import Rom
+from utils.fulltext import DEFAULT_FULLTEXT_SETTINGS
+
+
+@pytest.mark.parametrize(
+    ("dialect", "like_count", "has_match"),
+    [(MARIADB_DIALECT, 3, True), (POSTGRESQL_DIALECT, 9, False)],
+)
+def test_search_keeps_the_fulltext_index_for_indexable_words(
+    dialect: Dialect, like_count: int, has_match: bool
+):
+    with patch.object(
+        roms_handler, "fulltext_settings", return_value=DEFAULT_FULLTEXT_SETTINGS
+    ):
+        query = db_rom_handler._filter_by_search_term(select(Rom.id), "final fantasy 7")
+
+    sql = compile_sql(query, dialect, literal_binds=True)
+
+    assert ("AGAINST ('+final* +fantasy*' IN BOOLEAN MODE)" in sql) is has_match
+    # MariaDB checks only "7" by LIKE; PostgreSQL checks every word.
+    assert sql.count("LIKE") == like_count
+
+
+def test_search_uses_like_alone_while_the_settings_are_unreadable():
+    with patch.object(roms_handler, "fulltext_settings", return_value=None):
+        query = db_rom_handler._filter_by_search_term(select(Rom.id), "final fantasy")
+
+    sql = compile_sql(query, MARIADB_DIALECT, literal_binds=True)
+
+    assert "+final*" not in sql
+    assert sql.count("LIKE") == 6

backend/tests/handler/test_db_handler.py

@@ -254,6 +254,41 @@ def test_filter_by_search_term_multi_word_and_ranking(platform: Platform):
     assert explicit_ids.index(fantasy_final.id) < explicit_ids.index(ff.id)
 
 
+@pytest.mark.parametrize(
+    ("search_term", "expected"),
+    [
+        # "7" is too short for the FULLTEXT index.
+        ("final fantasy 7", {"Final Fantasy 7"}),
+        # "the" is an InnoDB stopword, "of" both short and a stopword.
+        ("the legend zelda", {"The Legend of Zelda"}),
+        ("legend of zelda", {"The Legend of Zelda"}),
+        # No word the index holds, so LIKE alone decides.
+        ("x-2", {"Final Fantasy X-2"}),
+        # The index holds "dr." as "dr", too short to require.
+        ("dr. mario", {"Dr. Mario"}),
+        # `_` is literal, not a LIKE wildcard.
+        ("x_2", set()),
+    ],
+)
+def test_filter_by_search_term_words_fulltext_cannot_index(
+    platform: Platform, search_term: str, expected: set[str]
+):
+    for name in (
+        "Final Fantasy 7",
+        "Final Fantasy X-2",
+        "Final Fantasy",
+        "Dr. Mario",
+        "The Legend of Zelda",
+        "Zelda II The Adventure of Link",
+    ):
+        # Spaced like a real dump, so the filename splits into the same words.
+        make_rom(platform, name, fs_stem=f"{name} (USA)")
+
+    results = db_rom_handler.get_roms_scalar(search_term=search_term)
+
+    assert {r.name for r in results} == expected
+
+
 def test_sibling_roms_empty_fs_name_no_tags_not_matched(platform: Platform):
     """ROMs with empty fs_name_no_tags should NOT be matched as siblings.
 

backend/tests/utils/test_fulltext.py

@@ -0,0 +1,146 @@
+from collections.abc import Iterator
+from unittest.mock import MagicMock
+
+import pytest
+from sqlalchemy import text
+from sqlalchemy.exc import OperationalError
+
+from handler.database.base_handler import sync_engine
+from utils import fulltext
+from utils.database import is_postgresql
+from utils.fulltext import (
+    DEFAULT_FULLTEXT_SETTINGS,
+    FulltextSettings,
+    fulltext_settings,
+    read_fulltext_settings,
+    split_fulltext_words,
+)
+
+IS_FULLTEXT_ENGINE = not is_postgresql(sync_engine)
+
+
+@pytest.fixture(autouse=True)
+def fresh_settings() -> Iterator[None]:
+    fulltext._server_fulltext_settings.cache_clear()
+    yield
+    fulltext._server_fulltext_settings.cache_clear()
+
+
+def _conn_reporting(
+    min_token_size: int = 3,
+    *,
+    stopwords_enabled: bool = True,
+    user_table: str | None = None,
+    server_table: str | None = None,
+) -> MagicMock:
+    conn = MagicMock()
+    conn.execute.return_value.one.return_value = (
+        min_token_size,
+        84,
+        int(stopwords_enabled),
+        user_table,
+        server_table,
+    )
+    conn.scalars.return_value = ["Foo", "bar", None]
+    return conn
+
+
+def _engine_connecting(conn: MagicMock) -> MagicMock:
+    engine = MagicMock()
+    engine.engine.name = "mariadb"
+    engine.connect.return_value.__enter__.return_value = conn
+    return engine
+
+
+@pytest.mark.skipif(not IS_FULLTEXT_ENGINE, reason="InnoDB full-text only")
+def test_reads_the_servers_settings():
+    with sync_engine.connect() as conn:
+        min_token_size = conn.execute(
+            text("SELECT @@innodb_ft_min_token_size")
+        ).scalar_one()
+        settings = read_fulltext_settings(conn)
+
+    assert settings.min_token_size == min_token_size
+    assert "the" in settings.stopwords
+
+
+@pytest.mark.skipif(IS_FULLTEXT_ENGINE, reason="PostgreSQL has no FULLTEXT index")
+def test_postgresql_uses_the_defaults():
+    assert fulltext_settings(sync_engine) == DEFAULT_FULLTEXT_SETTINGS
+
+
+def test_skips_fulltext_and_retries_when_the_server_is_unreachable():
+    engine = _engine_connecting(MagicMock())
+    engine.connect.side_effect = OperationalError("SELECT", {}, Exception("2013"))
+
+    assert fulltext_settings(engine) is None
+    assert fulltext_settings(engine) is None
+
+    assert engine.connect.call_count == 2
+
+
+def test_keeps_the_token_sizes_when_the_stopwords_are_unreadable():
+    conn = _conn_reporting(4)
+    conn.scalars.side_effect = OperationalError("SELECT", {}, Exception("1227"))
+
+    assert read_fulltext_settings(conn) == DEFAULT_FULLTEXT_SETTINGS._replace(
+        min_token_size=4
+    )
+
+
+def test_reads_a_custom_stopword_table():
+    conn = _conn_reporting(2, server_table="romm/stopwords")
+
+    settings = read_fulltext_settings(conn)
+
+    query = conn.scalars.call_args.args[0]
+    assert "FROM romm.stopwords" in str(query)
+    assert settings == FulltextSettings(2, 84, frozenset({"foo", "bar"}))
+
+
+def test_prefers_the_user_stopword_table():
+    conn = _conn_reporting(user_table="romm/mine", server_table="romm/stopwords")
+
+    read_fulltext_settings(conn)
+
+    assert "FROM romm.mine" in str(conn.scalars.call_args.args[0])
+
+
+def test_unreadable_custom_stopwords_skip_fulltext_and_retry():
+    conn = _conn_reporting(server_table="romm/stopwords")
+    conn.scalars.side_effect = OperationalError("SELECT", {}, Exception("1142"))
+
+    engine = _engine_connecting(conn)
+
+    assert fulltext_settings(engine) is None
+    assert fulltext_settings(engine) is None
+    assert conn.scalars.call_count == 2
+
+
+def test_disabled_stopwords_leave_none():
+    conn = _conn_reporting(stopwords_enabled=False)
+
+    assert read_fulltext_settings(conn) == FulltextSettings(3, 84, frozenset())
+
+
+@pytest.mark.parametrize(
+    ("term", "expected"),
+    [
+        ("final fantasy 7", (["final", "fantasy"], ["7"])),
+        ("the legend", (["legend"], ["the"])),
+        # InnoDB reads "dr." as the 2-letter token "dr".
+        ("dr. mario", (["mario"], ["dr."])),
+        ("pokemon: red", (["pokemon", "red"], [])),
+        ("spider-man x-2", (["spider", "man"], ["x-2"])),
+        ("mario's", (["mario's"], [])),
+        ("a" * 85, ([], ["a" * 85])),
+    ],
+)
+def test_split_fulltext_words(term: str, expected: tuple[list[str], list[str]]):
+    assert split_fulltext_words(term.split(), DEFAULT_FULLTEXT_SETTINGS) == expected
+
+
+def test_split_fulltext_words_follows_the_token_size():
+    settings = DEFAULT_FULLTEXT_SETTINGS._replace(min_token_size=2)
+
+    assert split_fulltext_words(["ff", "of"], settings) == (["ff"], ["of"])

backend/utils/fulltext.py

@@ -0,0 +1,103 @@
+"""InnoDB's full-text settings, and how its parser splits search words."""
+
+import functools
+import re
+from collections.abc import Iterable
+from typing import NamedTuple
+
+from sqlalchemy import Connection, Engine, String
+from sqlalchemy import column as sql_column
+from sqlalchemy import select
+from sqlalchemy import table as sql_table
+from sqlalchemy import text
+from sqlalchemy.exc import SQLAlchemyError
+
+from logger.logger import log
+from utils.database import is_postgresql
+
+# The tokens InnoDB's parser reads out of a word: "dr." holds only "dr".
+FULLTEXT_TOKEN_REGEX = re.compile(r"\w+(?:'\w+)*")
+
+
+class FulltextSettings(NamedTuple):
+    min_token_size: int
+    max_token_size: int
+    stopwords: frozenset[str]
+
+
+# InnoDB's defaults, for what the server won't report. The index skips
+# stopwords, so a required `+the*` would match nothing.
+DEFAULT_FULLTEXT_SETTINGS = FulltextSettings(
+    min_token_size=3,
+    max_token_size=84,
+    stopwords=frozenset(
+        "a about an are as at be by com de en for from how i in is it la of on or"
+        " that the this to und was what when where who will with www".split()
+    ),
+)
+
+
+def read_fulltext_settings(conn: Connection) -> FulltextSettings:
+    """The server's InnoDB full-text settings; raises if they can't be read."""
+    min_size, max_size, stopwords_enabled, user_table, server_table = conn.execute(
+        text(
+            "SELECT @@innodb_ft_min_token_size, @@innodb_ft_max_token_size,"
+            " @@innodb_ft_enable_stopword, @@innodb_ft_user_stopword_table,"
+            " @@innodb_ft_server_stopword_table"
+        )
+    ).one()
+    stopwords: frozenset[str] = frozenset()
+    if stopwords_enabled:
+        custom_table = user_table or server_table
+        schema, _, name = (
+            custom_table or "information_schema/INNODB_FT_DEFAULT_STOPWORD"
+        ).rpartition("/")
+        query = select(sql_column("value", String)).select_from(
+            sql_table(name, schema=schema or None)
+        )
+        try:
+            stopwords = frozenset(word.lower() for word in conn.scalars(query) if word)
+        except SQLAlchemyError as exc:
+            if custom_table:
+                raise
+            # MySQL needs PROCESS for INNODB_FT_DEFAULT_STOPWORD, which holds these.
+            log.warning(f"Using InnoDB's default full-text stopwords: {exc}")
+            stopwords = DEFAULT_FULLTEXT_SETTINGS.stopwords
+    return FulltextSettings(int(min_size), int(max_size), stopwords)
+
+
+@functools.cache
+def _server_fulltext_settings(engine: Engine) -> FulltextSettings:
+    with engine.connect() as conn:
+        return read_fulltext_settings(conn)
+
+
+def fulltext_settings(engine: Engine) -> FulltextSettings | None:
+    """The server's InnoDB full-text settings, or None while they can't be read."""
+    if is_postgresql(engine):
+        return DEFAULT_FULLTEXT_SETTINGS
+    try:
+        return _server_fulltext_settings(engine)
+    except SQLAlchemyError as exc:
+        # Left uncached, so a transient failure is retried on the next search.
+        log.warning(f"Can't read the full-text settings: {exc}")
+        return None
+
+
+def split_fulltext_words(
+    words: Iterable[str], settings: FulltextSettings
+) -> tuple[list[str], list[str]]:
+    """Words as (tokens a FULLTEXT index holds, words it can't hold)."""
+    indexed: list[str] = []
+    unindexed: list[str] = []
+    for word in words:
+        tokens = FULLTEXT_TOKEN_REGEX.findall(word)
+        if tokens and all(
+            settings.min_token_size <= len(token) <= settings.max_token_size
+            and token.lower() not in settings.stopwords
+            for token in tokens
+        ):
+            indexed.extend(tokens)
+        else:
+            unindexed.append(word)
+    return indexed, unindexed

docs/BACKEND_ARCHITECTURE.md

@@ -439,6 +439,10 @@ Query SQL that differs between engines lives in `utils/sql_dialect.py`
 (`DialectCase` and the helpers built on it), which picks each engine's spelling
 when the statement compiles. Handlers don't branch on `ROMM_DB_DRIVER`.
 
+`utils/fulltext.py` reads InnoDB's full-text token sizes and stopwords from the
+server and splits search words the way its parser does, so a search can require
+the words a FULLTEXT index holds and check the rest with `LIKE`.
+
 ### Engine & Session Setup
 
 **Location:** `handler/database/base_handler.py`