FIX: accept timeout=0 in bulkcopy as no timeout

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.

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"

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")