diff --git a/mssql_python/cursor.py b/mssql_python/cursor.py index d8c2c3b0..49d63529 100644 --- a/mssql_python/cursor.py +++ b/mssql_python/cursor.py @@ -3343,10 +3343,10 @@ def _bulkcopy_core_and_validate(table_name, batch_size, timeout): if batch_size < 0: raise ValueError(f"batch_size must be non-negative, got {batch_size}") - if not isinstance(timeout, int): - raise TypeError(f"timeout must be a positive integer, got {type(timeout).__name__}") - if timeout <= 0: - raise ValueError(f"timeout must be positive, got {timeout}") + if not isinstance(timeout, int) or isinstance(timeout, bool): + raise TypeError(f"timeout must be a non-negative integer, got {type(timeout).__name__}") + if timeout < 0: + raise ValueError(f"timeout must be non-negative, got {timeout}") return mssql_py_core @@ -3403,7 +3403,8 @@ def bulkcopy( batch_size: Number of rows to send per batch. Default 0 uses server optimal. - timeout: Operation timeout in seconds. Default is 30. + timeout: Operation timeout in seconds. Default is 30. 0 disables the + bulk copy operation timeout. column_mappings: Maps source data columns to target table column names. Two formats supported: @@ -3590,7 +3591,8 @@ def bulkcopy_arrow( table_name: Target table name (may include schema, e.g. 'dbo.MyTable'). source: Arrow source (see above). batch_size: Rows per TDS commit. Default 0 uses server optimal. - timeout: Operation timeout in seconds. Default 30. + timeout: Operation timeout in seconds. Default 30. 0 disables the + bulk copy operation timeout. column_mappings: Same two formats as :meth:`bulkcopy`. When omitted, Arrow fields map to destination columns by ordinal position. keep_identity: Preserve identity values from the source. diff --git a/tests/test_019_bulkcopy.py b/tests/test_019_bulkcopy.py index 696e4901..05fa7d5c 100644 --- a/tests/test_019_bulkcopy.py +++ b/tests/test_019_bulkcopy.py @@ -3,6 +3,8 @@ """Basic integration tests for bulkcopy via the mssql_python driver.""" +import time + import pytest # Skip the entire module when mssql_py_core can't be loaded (e.g. it's not @@ -440,6 +442,78 @@ def test_bulkcopy_tuple_passthrough(cursor): cursor.connection.commit() +def test_bulkcopy_accepts_timeout_zero(cursor): + """GH-697: timeout=0 means no timeout per the BCP spec, so it must not be rejected.""" + table_name = "mssql_python_bcp_timeout_zero" + try: + cursor.execute(f"IF OBJECT_ID('{table_name}', 'U') IS NOT NULL DROP TABLE {table_name}") + cursor.execute(f"CREATE TABLE {table_name} (id INT, name VARCHAR(20))") + cursor.connection.commit() + + result = cursor.bulkcopy(table_name, [(1, "Alice"), (2, "Bob")], timeout=0) + assert result["rows_copied"] == 2 + + cursor.execute(f"SELECT COUNT(*) FROM {table_name}") + assert cursor.fetchone()[0] == 2 + + finally: + cursor.execute(f"IF OBJECT_ID('{table_name}', 'U') IS NOT NULL DROP TABLE {table_name}") + cursor.connection.commit() + + +def test_bulkcopy_timeout_zero_disables_the_timeout(cursor): + """GH-697: a copy that times out at timeout=1 completes at timeout=0. + + The source paces itself with sleeps so the copy always outlives a 1 second + timeout regardless of machine speed. Same source both times, so the only + variable is the timeout value. + """ + table_name = "mssql_python_bcp_timeout_zero_behaviour" + + def slow_rows(): + for i in range(30): + time.sleep(0.1) # 3s total, comfortably past the 1s timeout + yield (i, f"row{i}") + + try: + cursor.execute(f"IF OBJECT_ID('{table_name}', 'U') IS NOT NULL DROP TABLE {table_name}") + cursor.execute(f"CREATE TABLE {table_name} (id INT, name NVARCHAR(50))") + cursor.connection.commit() + + # baseline: the timeout is real and fires on this source + with pytest.raises(Exception, match="(?i)timeout"): + cursor.bulkcopy(table_name, slow_rows(), timeout=1) + + cursor.execute(f"DELETE FROM {table_name}") + cursor.connection.commit() + + # same source, timeout disabled: runs to completion + result = cursor.bulkcopy(table_name, slow_rows(), timeout=0) + assert result["rows_copied"] == 30 + + cursor.execute(f"SELECT COUNT(*) FROM {table_name}") + assert cursor.fetchone()[0] == 30 + + finally: + cursor.execute(f"IF OBJECT_ID('{table_name}', 'U') IS NOT NULL DROP TABLE {table_name}") + cursor.connection.commit() + + +def test_bulkcopy_rejects_invalid_timeouts(cursor): + """GH-697: negatives and bools stay rejected. Validation fires before any DB work.""" + with pytest.raises(ValueError, match="timeout must be non-negative"): + cursor.bulkcopy("mssql_python_bcp_unused", [(1,)], timeout=-1) + + with pytest.raises(TypeError, match="timeout must be a non-negative integer"): + cursor.bulkcopy("mssql_python_bcp_unused", [(1,)], timeout=1.5) + + # bool is an int subclass, so False would otherwise slip through as 0 + # and silently disable the timeout. + for flag in (False, True): + with pytest.raises(TypeError, match="timeout must be a non-negative integer"): + cursor.bulkcopy("mssql_python_bcp_unused", [(1,)], timeout=flag) + + def test_bulkcopy_empty_iterator(cursor): """Regression test: empty iterator doesn't crash (GH-482).""" table_name = "mssql_python_bcp_empty" diff --git a/tests/test_024_bulkcopy_arrow.py b/tests/test_024_bulkcopy_arrow.py index 7195606b..a52ead3e 100644 --- a/tests/test_024_bulkcopy_arrow.py +++ b/tests/test_024_bulkcopy_arrow.py @@ -206,9 +206,16 @@ def test_timeout_wrong_type(self): with pytest.raises(TypeError, match="timeout"): _bare_cursor().bulkcopy_arrow("t", pa.table({"a": [1]}), timeout="30") - def test_timeout_non_positive(self): + def test_timeout_negative(self): with pytest.raises(ValueError, match="timeout"): - _bare_cursor().bulkcopy_arrow("t", pa.table({"a": [1]}), timeout=0) + _bare_cursor().bulkcopy_arrow("t", pa.table({"a": [1]}), timeout=-1) + + def test_timeout_bool_rejected(self): + # bool is an int subclass, so False would otherwise slip through as 0 + # and silently disable the timeout. + for flag in (False, True): + with pytest.raises(TypeError, match="timeout"): + _bare_cursor().bulkcopy_arrow("t", pa.table({"a": [1]}), timeout=flag) def test_missing_pycore_raises_importerror(self): cur = _bare_cursor() @@ -484,6 +491,22 @@ def test_table_round_trip_with_nulls(self, cursor): assert rows[2][2] is None cursor.execute(f"DROP TABLE {t}") + def test_timeout_zero_copies(self, cursor): + """GH-697: timeout=0 means no timeout, so a copy with it must succeed.""" + t = "mssql_python_arrow_timeout_zero" + _make_table(cursor, t, "id INT NOT NULL, name NVARCHAR(50) NULL") + tbl = pa.table( + { + "id": pa.array([1, 2, 3], type=pa.int32()), + "name": pa.array(["a", "b", "c"]), + } + ) + result = cursor.bulkcopy_arrow(t, tbl, timeout=0) + assert result["rows_copied"] == 3 + cursor.execute(f"SELECT COUNT(*) FROM {t}") + assert cursor.fetchone()[0] == 3 + cursor.execute(f"DROP TABLE {t}") + def test_record_batch_source(self, cursor): t = "mssql_python_arrow_batch" _make_table(cursor, t, "id INT NOT NULL")