Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions core/wren/src/wren/policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,14 @@ def resolve_model_name(
case-sensitively; an unquoted identifier prefers an exact case match but
falls back to a case-insensitive scan. Returns ``None`` if no model
matches.

When an unquoted reference matches more than one model case-insensitively
(e.g. both ``Users`` and ``users`` are defined), the one picked is the
lexicographically smallest rather than whichever ``model_names`` happens
to iterate first: for a ``set`` of strings that iteration order is not
the insertion order, it depends on string hashing and varies across
processes under ``PYTHONHASHSEED``, so the same query could silently
bind a different model on every run.
"""
model_set = (
model_names if isinstance(model_names, (set, frozenset)) else set(model_names)
Expand All @@ -170,10 +178,14 @@ def resolve_model_name(
if quoted:
return None
name_lower = name.lower()
# Single pass, no intermediate list: a tie only needs `<` against the
# best candidate seen so far, same cost as the exact-match scan this
# replaces.
best: str | None = None
for candidate in model_set:
if candidate.lower() == name_lower:
return candidate
return None
if candidate.lower() == name_lower and (best is None or candidate < best):
best = candidate
return best


# Statement shapes that are read-only queries. ``SetOperation`` is the base class
Expand Down
56 changes: 55 additions & 1 deletion core/wren/tests/unit/test_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@

from __future__ import annotations

import os
import subprocess
import sys

import pytest
from sqlglot import exp, parse_one

from wren.config import WrenConfig
from wren.model.error import ErrorCode, WrenError
from wren.policy import validate_planned_sql, validate_sql_policy
from wren.policy import resolve_model_name, validate_planned_sql, validate_sql_policy

pytestmark = pytest.mark.unit

Expand Down Expand Up @@ -669,3 +673,53 @@ def test_planned_sql_fails_open_when_unparseable():
open where the input direction fails closed.
"""
validate_planned_sql("SELCT a FRM orders", "postgres")


# ── resolve_model_name, case-insensitive fallback ──────────────────────────


def test_resolve_model_name_exact_match_wins():
assert resolve_model_name("orders", False, {"orders", "Orders"}) == "orders"


def test_resolve_model_name_quoted_is_case_sensitive():
assert resolve_model_name("Orders", True, {"orders"}) is None


def test_resolve_model_name_unquoted_case_insensitive_fallback():
assert resolve_model_name("ORDERS", False, {"orders"}) == "orders"


def test_resolve_model_name_no_match_returns_none():
assert resolve_model_name("missing", False, {"orders"}) is None


def test_resolve_model_name_case_collision_is_deterministic():
"""When two model names differ only in case, an unquoted reference that
matches neither exactly must resolve the same way every time.

Before the fix this depended on ``set`` iteration order, which varies by
process under ``PYTHONHASHSEED``, not within one: a fixed hash seed makes
iteration order stable for the lifetime of a process, so a same-process
loop passes on the buggy code too, every time. The bug only shows up
across fresh interpreters, so the regression spawns one per
``PYTHONHASHSEED`` instead, matching how this was reproduced originally
(8 fresh subprocesses split 6/2 between ``'Orders'`` and ``'orders'``).
Sorted order is an arbitrary tie-break (the two names are equally
"right", this pair may legitimately be two distinct models, see
test_case_sensitivity.py); the point is only that it no longer varies.
"""
script = (
"from wren.policy import resolve_model_name; "
"print(resolve_model_name('ORDERS', False, {'Orders', 'orders'}))"
)
for seed in range(8):
env = {**os.environ, "PYTHONHASHSEED": str(seed)}
result = subprocess.run(
[sys.executable, "-c", script],
env=env,
capture_output=True,
text=True,
check=True,
)
assert result.stdout.strip() == "Orders"
Loading