From f5e5f447af95daaf986e373698ac252f858a7dd2 Mon Sep 17 00:00:00 2001 From: kert Date: Wed, 11 Nov 2026 19:31:49 -0500 Subject: [PATCH] fix(llm): skip the database-wide iterative_scan ALTER once it is in place hnsw.iterative_scan = relaxed_order is already set on the llm database (pg_db_role_setting), but migrate re-issued ALTER DATABASE on every index run as the llm role, which may not, and postgres logged two permission ERRORs each time. migrate now checks the setting first. Verified live: the check sees it, a migrate run logs nothing. --- src/llm/migrate.py | 10 ++++++++++ tests/llm/test_migrate.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/llm/migrate.py b/src/llm/migrate.py index 8f383bc..69fdf21 100644 --- a/src/llm/migrate.py +++ b/src/llm/migrate.py @@ -89,6 +89,12 @@ _METADATA_INDEX_DDL = [ # — the API's pooled connections included — inherits it without having # to load the extension's GUCs first. ITERATIVE_SCAN_DDL = "ALTER DATABASE {db} SET hnsw.iterative_scan = 'relaxed_order'" +_ITERATIVE_SCAN_SET_SQL = ( + "SELECT EXISTS (SELECT 1 FROM pg_db_role_setting s " + "JOIN pg_database d ON d.oid = s.setdatabase " + "WHERE d.datname = current_database() AND s.setrole = 0 " + "AND 'hnsw.iterative_scan=relaxed_order' = ANY (s.setconfig))" +) def migrate(engine: Engine) -> None: @@ -109,6 +115,10 @@ def ensure_iterative_scan(engine: Engine) -> None: try: with engine.begin() as conn: db = conn.execute(text("SELECT current_database()")).scalar() + # Already in place (set once by a role that may): re-issuing it + # as the llm role only logs a permission ERROR in postgres. + if conn.execute(text(_ITERATIVE_SCAN_SET_SQL)).scalar(): + return conn.execute(text(ITERATIVE_SCAN_DDL.format(db=db))) except Exception as exc: # noqa: BLE001 — the llm role usually may not ALTER DATABASE # every connection also SETs it itself (llm.index.set_iterative_scan), diff --git a/tests/llm/test_migrate.py b/tests/llm/test_migrate.py index 9f5a52a..631a26f 100644 --- a/tests/llm/test_migrate.py +++ b/tests/llm/test_migrate.py @@ -88,7 +88,15 @@ class TestIterativeScan: engine = MagicMock() conn = engine.begin.return_value.__enter__.return_value - conn.execute.return_value.scalar.return_value = "llm" + + def answer(stmt, *a, **k): + r = MagicMock() + r.scalar.return_value = ( + False if "pg_db_role_setting" in str(stmt) else "llm" + ) + return r + + conn.execute.side_effect = answer migrate(engine) sql = [str(c.args[0]) for c in conn.execute.call_args_list] assert any( @@ -96,6 +104,26 @@ class TestIterativeScan: for q in sql ) + def test_already_set_database_wide_issues_no_alter(self): + # The llm role may not ALTER DATABASE; re-issuing it on every index + # run only logged two permission ERRORs in postgres per run. + from unittest.mock import MagicMock + + from llm.migrate import ensure_iterative_scan + + engine = MagicMock() + conn = engine.begin.return_value.__enter__.return_value + + def answer(stmt, *a, **k): + r = MagicMock() + r.scalar.return_value = True if "pg_db_role_setting" in str(stmt) else "llm" + return r + + conn.execute.side_effect = answer + ensure_iterative_scan(engine) + sql = [str(c.args[0]) for c in conn.execute.call_args_list] + assert not any("ALTER DATABASE" in q for q in sql) + def test_failure_is_a_warning_not_an_error(self, caplog): from unittest.mock import MagicMock