Skip to content

Commit a04832b

Browse files
zahariashclaude
authored andcommitted
fix(connection): validate settings in the setter and keep one timeout value
set_query_timeout() and set_max_threads_for_exec() now check the value before queueing it: an integer (TypeError otherwise) between 0 and 2**64 - 1 (ValueError otherwise). Only the native call is deferred to the next query. Before, a bad value surfaced on the next execute() on the pybind backend (TypeError from the uint64 conversion), and the C-API backend wrapped negative values into huge ones. Negative values now raise ValueError on both backends. The connection no longer keeps its own copy of the timeout: the C-API short-circuit reads the backend connection's value, which is set when the native setter runs, so both backends see the timeout from the same point. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent d1d0462 commit a04832b

2 files changed

Lines changed: 53 additions & 6 deletions

File tree

‎src_py/connection.py‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import functools
55
import inspect
66
import json
7+
import operator
78
import re
89
import threading
910
import uuid
@@ -385,6 +386,15 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
385386
return wrapper
386387

387388

389+
def _setting_value(name: str, value: int) -> int:
390+
"""Validate a connection setting before it is queued; the engine takes a uint64."""
391+
value = operator.index(value)
392+
if not 0 <= value < 2**64:
393+
msg = f"{name} must be between 0 and 2**64 - 1; found {value}."
394+
raise ValueError(msg)
395+
return value
396+
397+
388398
class Connection:
389399
"""Connection to a database."""
390400

@@ -410,7 +420,6 @@ def __init__(self, database: Database, num_threads: int = 0):
410420
self._calls = threading.Condition()
411421
self._calls_in_flight = 0
412422
self._prefer_pybind = False
413-
self._query_timeout_ms = 0
414423
# Native setter name -> value, applied before the next query.
415424
self._pending_settings: dict[str, int] = {}
416425
self._pending_settings_lock = threading.Lock()
@@ -474,12 +483,21 @@ def set_max_threads_for_exec(self, num_threads: int) -> None:
474483
Parameters
475484
----------
476485
num_threads : int
477-
Maximum number of threads to use for executing queries.
486+
Maximum number of threads to use for executing queries; 0 uses the
487+
database's limit.
488+
489+
Raises
490+
------
491+
TypeError
492+
If `num_threads` is not an integer.
493+
ValueError
494+
If `num_threads` is negative or does not fit in 64 bits.
478495
479496
"""
497+
num_threads = _setting_value("num_threads", num_threads)
480498
self.init_connection()
481499
with self._pending_settings_lock:
482-
self._pending_settings["set_max_threads_for_exec"] = int(num_threads)
500+
self._pending_settings["set_max_threads_for_exec"] = num_threads
483501

484502
def _apply_pending_settings(self) -> None:
485503
# The engine applies settings under the lock a running query holds, so
@@ -1005,7 +1023,7 @@ def execute(
10051023

10061024
if (
10071025
not self._using_pybind_backend()
1008-
and self._query_timeout_ms > 0
1026+
and self._connection._query_timeout_ms > 0
10091027
and isinstance(query, str)
10101028
and len(re.findall(r"(?i)\bUNWIND\s+RANGE\s*\(", query)) >= 2
10111029
):
@@ -1213,11 +1231,18 @@ def set_query_timeout(self, timeout_in_ms: int) -> None:
12131231
timeout_in_ms : int
12141232
query timeout value in ms for executing queries.
12151233
1234+
Raises
1235+
------
1236+
TypeError
1237+
If `timeout_in_ms` is not an integer.
1238+
ValueError
1239+
If `timeout_in_ms` is negative or does not fit in 64 bits.
1240+
12161241
"""
1242+
timeout_in_ms = _setting_value("timeout_in_ms", timeout_in_ms)
12171243
self.init_connection()
1218-
self._query_timeout_ms = int(timeout_in_ms)
12191244
with self._pending_settings_lock:
1220-
self._pending_settings["set_query_timeout"] = self._query_timeout_ms
1245+
self._pending_settings["set_query_timeout"] = timeout_in_ms
12211246

12221247
def interrupt(self) -> None:
12231248
"""

‎test/test_timeout.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,25 @@ def test_applying_settings_lets_other_threads_run(conn: lb.Connection) -> None:
8080
elapsed = time.monotonic() - started
8181
_interrupt_until_done(conn, running, waiting)
8282
assert elapsed < 1, elapsed
83+
84+
85+
@pytest.mark.parametrize("setter", ["set_query_timeout", "set_max_threads_for_exec"])
86+
@pytest.mark.parametrize(
87+
("value", "error"),
88+
[("1", TypeError), (1.5, TypeError), (-1, ValueError), (2**64, ValueError)],
89+
)
90+
def test_settings_reject_invalid_values_at_once(
91+
conn: lb.Connection, setter: str, value: object, error: type[Exception]
92+
) -> None:
93+
with pytest.raises(error):
94+
getattr(conn, setter)(value)
95+
assert conn.execute("RETURN 1").get_all() == [[1]]
96+
97+
98+
def test_cleared_timeout_applies_to_next_query(conn: lb.Connection) -> None:
99+
query = "UNWIND range(1, 10) AS x UNWIND range(1, 10) AS y RETURN count(*)"
100+
conn.set_query_timeout(1000)
101+
with contextlib.suppress(RuntimeError):
102+
conn.execute(query)
103+
conn.set_query_timeout(0)
104+
assert conn.execute(query).get_all() == [[100]]

0 commit comments

Comments
 (0)