From a31136cd2121aa74356a5083a6fa147eb04b5754 Mon Sep 17 00:00:00 2001 From: Sam Dornan Date: Wed, 30 Sep 2026 22:45:44 -0400 Subject: [PATCH] Keep the FULLTEXT index on searches with short words or stopwords (#4970) --- backend/handler/database/roms_handler.py | 79 +++++----- .../database/test_roms_fulltext_search.py | 39 +++++ backend/tests/handler/test_db_handler.py | 35 +++++ backend/tests/utils/test_fulltext.py | 146 ++++++++++++++++++ backend/utils/fulltext.py | 103 ++++++++++++ docs/BACKEND_ARCHITECTURE.md | 4 + 6 files changed, 362 insertions(+), 44 deletions(-) create mode 100644 backend/tests/handler/database/test_roms_fulltext_search.py create mode 100644 backend/tests/utils/test_fulltext.py create mode 100644 backend/utils/fulltext.py diff --git a/backend/handler/database/roms_handler.py b/backend/handler/database/roms_handler.py index ba358945b..2a155f2d7 100644 --- a/backend/handler/database/roms_handler.py +++ b/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. diff --git a/backend/tests/handler/database/test_roms_fulltext_search.py b/backend/tests/handler/database/test_roms_fulltext_search.py new file mode 100644 index 000000000..c459152f8 --- /dev/null +++ b/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 diff --git a/backend/tests/handler/test_db_handler.py b/backend/tests/handler/test_db_handler.py index 974b9186d..c2f2c19ca 100644 --- a/backend/tests/handler/test_db_handler.py +++ b/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. diff --git a/backend/tests/utils/test_fulltext.py b/backend/tests/utils/test_fulltext.py new file mode 100644 index 000000000..642f3c90c --- /dev/null +++ b/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"]) diff --git a/backend/utils/fulltext.py b/backend/utils/fulltext.py new file mode 100644 index 000000000..51151c23b --- /dev/null +++ b/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 diff --git a/docs/BACKEND_ARCHITECTURE.md b/docs/BACKEND_ARCHITECTURE.md index f37217f53..519266b9e 100644 --- a/docs/BACKEND_ARCHITECTURE.md +++ b/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`