mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
fix(storage): survive oversized rows in postgres history search
to_tsvector was computed inline over full row content, so one row whose tsvector exceeds PostgreSQL's 1MB limit aborted every search_history scan. Cap the FTS input at 250K chars (worst-case tsvector expansion stays under the limit; giant rows remain findable by their head). The ILIKE fallback also never ran on postgres: the failed statement leaves the autobegun transaction aborted, so roll it back before falling back.
This commit is contained in:
@@ -4,6 +4,7 @@ from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
import sqlalchemy as sa
|
||||
|
||||
from turnstone.core.storage._schema import workstreams
|
||||
@@ -576,6 +577,48 @@ class TestSearch:
|
||||
results = backend.search_history_recent(limit=1)
|
||||
assert len(results) == 1
|
||||
|
||||
def test_search_history_survives_oversized_row(self, backend):
|
||||
# A multi-MB row of mostly-unique words: on PostgreSQL its full
|
||||
# tsvector exceeds the 1MB hard limit, which used to abort every
|
||||
# search_history scan ("string is too long for tsvector") — one
|
||||
# giant tool dump silently killed history recall entirely.
|
||||
backend.register_workstream("s1")
|
||||
giant = "gargantuan beacon " + " ".join(f"w{i}" for i in range(300_000))
|
||||
assert len(giant) > 2_000_000
|
||||
backend.save_message("s1", "tool", giant)
|
||||
backend.save_message("s1", "user", "hello world")
|
||||
|
||||
results = backend.search_history("hello")
|
||||
assert any("hello" in str(r[3]) for r in results)
|
||||
|
||||
# The oversized row itself stays findable by its head.
|
||||
results = backend.search_history("gargantuan beacon")
|
||||
assert any("gargantuan" in str(r[3]) for r in results)
|
||||
|
||||
def test_search_history_fts_error_falls_back_to_ilike(self, request, backend, monkeypatch):
|
||||
# PostgreSQL only: a failed FTS statement aborts the connection's
|
||||
# autobegun transaction, and the ILIKE fallback runs on that same
|
||||
# connection — without a rollback first it dies with
|
||||
# InFailedSqlTransaction instead of returning results.
|
||||
if request.config.getoption("--storage-backend") != "postgresql":
|
||||
pytest.skip("exercises PostgreSQL aborted-transaction fallback")
|
||||
|
||||
backend.register_workstream("s1")
|
||||
backend.save_message("s1", "user", "hello fallback world")
|
||||
|
||||
real_execute = sa.engine.Connection.execute
|
||||
|
||||
def failing_fts_execute(self, statement, *args, **kwargs):
|
||||
if "to_tsvector" in str(statement):
|
||||
# A genuine server-side error, so the transaction is aborted
|
||||
# exactly as when to_tsvector rejects a row.
|
||||
return real_execute(self, sa.text("SELECT 1/0"))
|
||||
return real_execute(self, statement, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(sa.engine.Connection, "execute", failing_fts_execute)
|
||||
results = backend.search_history("fallback")
|
||||
assert any("fallback" in str(r[3]) for r in results)
|
||||
|
||||
|
||||
# -- Workstream operations -----------------------------------------------------
|
||||
|
||||
|
||||
@@ -178,6 +178,13 @@ from turnstone.core.workstream import BULK_CLOSE_STATE_VALUES, WorkstreamKind
|
||||
|
||||
log = get_logger(__name__)
|
||||
|
||||
# PostgreSQL rejects any tsvector larger than 1MB ("string is too long for
|
||||
# tsvector"), and search_history computes tsvectors inline per row — so one
|
||||
# oversized row would abort the whole scan and every search with it. Worst
|
||||
# case a tsvector runs ~4x its input (unique short lexemes + position data),
|
||||
# so 250K chars keeps even pathological rows safely under the limit.
|
||||
_FTS_INPUT_CAP_CHARS = 250_000
|
||||
|
||||
|
||||
def _resolve_pg_listen_url(override: str, sqlalchemy_url: str) -> str:
|
||||
"""Resolve the URL used by the dedicated LISTEN connection.
|
||||
@@ -1289,26 +1296,31 @@ class PostgreSQLBackend:
|
||||
scope_params["excl_ws"] = exclude_ws_id
|
||||
scope_params["excl_after"] = -1 if exclude_after is None else exclude_after
|
||||
with self._conn() as conn:
|
||||
# Use PostgreSQL full-text search if search_vector column exists
|
||||
# Full-text search over an inline tsvector (there is no indexed
|
||||
# search_vector column). The input is capped — see
|
||||
# _FTS_INPUT_CAP_CHARS — so a single giant row (multi-MB tool
|
||||
# dumps exist) cannot trip PostgreSQL's 1MB tsvector limit and
|
||||
# abort every search; oversized rows stay findable by their head.
|
||||
try:
|
||||
return list(
|
||||
conn.execute(
|
||||
sa.text(
|
||||
"SELECT c.timestamp, c.ws_id, c.role, c.content, c.tool_name "
|
||||
"FROM conversations c "
|
||||
"WHERE to_tsvector('english', COALESCE(c.content, '')) "
|
||||
"WHERE to_tsvector('english', left(COALESCE(c.content, ''), :fts_cap)) "
|
||||
" @@ plainto_tsquery('english', :query) "
|
||||
# Exclude compaction-checkpoint markers (resume-only
|
||||
# summary artifacts); IS DISTINCT FROM is NULL-safe so
|
||||
# normal rows (_source NULL) are not dropped.
|
||||
"AND c._source IS DISTINCT FROM :compaction_source "
|
||||
+ scope_sql
|
||||
+ "ORDER BY ts_rank(to_tsvector('english', COALESCE(c.content, '')), "
|
||||
+ "ORDER BY ts_rank(to_tsvector('english', left(COALESCE(c.content, ''), :fts_cap)), "
|
||||
" plainto_tsquery('english', :query)) DESC "
|
||||
"LIMIT :limit OFFSET :offset"
|
||||
),
|
||||
{
|
||||
"query": query,
|
||||
"fts_cap": _FTS_INPUT_CAP_CHARS,
|
||||
"compaction_source": _COMPACTION_SOURCE,
|
||||
"limit": capped,
|
||||
"offset": capped_offset,
|
||||
@@ -1317,6 +1329,11 @@ class PostgreSQLBackend:
|
||||
).fetchall()
|
||||
)
|
||||
except Exception:
|
||||
# The failed statement aborted the connection's autobegun
|
||||
# transaction; PostgreSQL then refuses every command until a
|
||||
# rollback, so without this the fallback can never run
|
||||
# (InFailedSqlTransaction).
|
||||
conn.rollback()
|
||||
# Fallback to ILIKE
|
||||
return list(
|
||||
conn.execute(
|
||||
|
||||
Reference in New Issue
Block a user