Skip to content

Commit ba369c7

Browse files
authored
FIX: Invalid check for database in connection string in _bulkcopy (#445)
<!-- mssql-python maintainers: ADO Work Item --> > [AB#42753](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/42753) <!-- External contributors: GitHub Issue --> > GitHub Issue: #442 ------------------------------------------------------------------- ### Summary This pull request updates the bulk copy functionality to make the `DATABASE` parameter in the connection string optional, aligning behavior with SQL Server's default handling. It also adds a new test to ensure this works as expected. The most important changes are: **Bulk copy connection string handling:** * Modified the `_bulkcopy` method in `cursor.py` to remove the requirement for the `DATABASE` parameter in the connection string; now, bulk copy will proceed as long as a server is specified, and the server will use the default database if none is provided. **Testing improvements:** * Added a new test, `test_bulkcopy_without_database_parameter`, in `test_019_bulkcopy.py` to verify that bulk copy works correctly when the `DATABASE` parameter is omitted, ensuring the client connects to the default database and data is inserted as expected.
1 parent 0242a91 commit ba369c7

2 files changed

Lines changed: 216 additions & 7 deletions

File tree

mssql_python/cursor.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2624,15 +2624,10 @@ def bulkcopy(
26242624
parser = _ConnectionStringParser(validate_keywords=False)
26252625
params = parser._parse(self.connection.connection_str)
26262626

2627-
if not params.get("server"):
2627+
# Check for server parameter (accepts synonyms: server, addr, address)
2628+
if not (params.get("server") or params.get("addr") or params.get("address")):
26282629
raise ValueError("SERVER parameter is required in connection string")
26292630

2630-
if not params.get("database"):
2631-
raise ValueError(
2632-
"DATABASE parameter is required in connection string for bulk copy. "
2633-
"Specify the target database explicitly to avoid accidentally writing to system databases."
2634-
)
2635-
26362631
# Translate parsed connection string into the dict py-core expects.
26372632
pycore_context = connstr_to_pycore_params(params)
26382633

tests/test_019_bulkcopy.py

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,217 @@ def test_bulkcopy_basic(cursor):
8282

8383
# Cleanup
8484
cursor.execute(f"DROP TABLE {table_name}")
85+
86+
87+
def test_bulkcopy_without_database_parameter(conn_str):
88+
"""Test bulkcopy operation works when DATABASE is not specified in connection string.
89+
90+
The database keyword in connection string is optional. In its absence,
91+
the client sends an empty database name and the server responds with
92+
the default database the client was connected to.
93+
"""
94+
from mssql_python import connect
95+
from mssql_python.connection_string_parser import _ConnectionStringParser
96+
from mssql_python.connection_string_builder import _ConnectionStringBuilder
97+
98+
# Parse the connection string using the proper parser
99+
parser = _ConnectionStringParser(validate_keywords=False)
100+
params = parser._parse(conn_str)
101+
102+
# Save the original database name to use it explicitly in our operations
103+
original_database = params.get("database")
104+
105+
# Remove DATABASE parameter if present (case-insensitive, handles all synonyms)
106+
params.pop("database", None)
107+
108+
# Rebuild the connection string using the builder to preserve braced values
109+
builder = _ConnectionStringBuilder(params)
110+
conn_str_no_db = builder.build()
111+
112+
# Create connection without DATABASE parameter
113+
conn = connect(conn_str_no_db)
114+
try:
115+
cursor = conn.cursor()
116+
117+
# Verify we're connected to a database (should be the default)
118+
cursor.execute("SELECT DB_NAME() AS current_db")
119+
current_db = cursor.fetchone()[0]
120+
assert current_db is not None, "Should be connected to a database"
121+
122+
# If original database was specified, switch to it to ensure we have permissions
123+
if original_database:
124+
cursor.execute(f"USE [{original_database}]")
125+
126+
# Create test table in the current database
127+
table_name = "mssql_python_bulkcopy_no_db_test"
128+
cursor.execute(f"IF OBJECT_ID('{table_name}', 'U') IS NOT NULL DROP TABLE {table_name}")
129+
cursor.execute(f"CREATE TABLE {table_name} (id INT, name VARCHAR(50), value FLOAT)")
130+
conn.commit()
131+
132+
# Prepare test data
133+
data = [
134+
(1, "Alice", 100.5),
135+
(2, "Bob", 200.75),
136+
(3, "Charlie", 300.25),
137+
]
138+
139+
# Perform bulkcopy - this should NOT raise ValueError about missing DATABASE
140+
# Note: bulkcopy creates its own connection, so we need to use fully qualified table name
141+
# if we had a database in the original connection string
142+
bulkcopy_table_name = (
143+
f"[{original_database}].[dbo].{table_name}" if original_database else table_name
144+
)
145+
result = cursor.bulkcopy(bulkcopy_table_name, data, timeout=60)
146+
147+
# Verify result
148+
assert result is not None
149+
assert result["rows_copied"] == 3
150+
151+
# Verify data was inserted correctly
152+
cursor.execute(f"SELECT id, name, value FROM {table_name} ORDER BY id")
153+
rows = cursor.fetchall()
154+
155+
assert len(rows) == 3
156+
assert rows[0][0] == 1 and rows[0][1] == "Alice" and abs(rows[0][2] - 100.5) < 0.01
157+
assert rows[1][0] == 2 and rows[1][1] == "Bob" and abs(rows[1][2] - 200.75) < 0.01
158+
assert rows[2][0] == 3 and rows[2][1] == "Charlie" and abs(rows[2][2] - 300.25) < 0.01
159+
160+
# Cleanup
161+
cursor.execute(f"DROP TABLE {table_name}")
162+
cursor.close()
163+
finally:
164+
conn.close()
165+
166+
167+
def test_bulkcopy_with_server_synonyms(conn_str):
168+
"""Test that bulkcopy works with all SERVER parameter synonyms: server, addr, address."""
169+
from mssql_python import connect
170+
from mssql_python.connection_string_parser import _ConnectionStringParser
171+
from mssql_python.connection_string_builder import _ConnectionStringBuilder
172+
173+
# Parse the connection string using the proper parser
174+
parser = _ConnectionStringParser(validate_keywords=False)
175+
params = parser._parse(conn_str)
176+
177+
# Test with 'Addr' synonym - replace 'server' with 'addr'
178+
server_value = (
179+
params.pop("server", None) or params.pop("addr", None) or params.pop("address", None)
180+
)
181+
params["addr"] = server_value
182+
builder = _ConnectionStringBuilder(params)
183+
conn_string_addr = builder.build()
184+
185+
conn = connect(conn_string_addr)
186+
try:
187+
cursor = conn.cursor()
188+
table_name = "test_bulkcopy_addr_synonym"
189+
190+
# Create table
191+
cursor.execute(f"DROP TABLE IF EXISTS {table_name}")
192+
cursor.execute(f"""
193+
CREATE TABLE {table_name} (
194+
id INT,
195+
name NVARCHAR(50),
196+
value FLOAT
197+
)
198+
""")
199+
conn.commit()
200+
201+
# Test data
202+
test_data = [(1, "Test1", 1.5), (2, "Test2", 2.5), (3, "Test3", 3.5)]
203+
204+
# Perform bulkcopy with connection using Addr parameter
205+
result = cursor.bulkcopy(table_name, test_data)
206+
207+
# Verify result
208+
assert result is not None
209+
assert "rows_copied" in result
210+
assert result["rows_copied"] == 3
211+
212+
# Verify data
213+
cursor.execute(f"SELECT COUNT(*) FROM {table_name}")
214+
count = cursor.fetchone()[0]
215+
assert count == 3
216+
217+
# Cleanup
218+
cursor.execute(f"DROP TABLE {table_name}")
219+
cursor.close()
220+
finally:
221+
conn.close()
222+
223+
# Test with 'Address' synonym - replace with 'address'
224+
params = parser._parse(conn_str)
225+
server_value = (
226+
params.pop("server", None) or params.pop("addr", None) or params.pop("address", None)
227+
)
228+
params["address"] = server_value
229+
builder = _ConnectionStringBuilder(params)
230+
conn_string_address = builder.build()
231+
232+
conn = connect(conn_string_address)
233+
try:
234+
cursor = conn.cursor()
235+
table_name = "test_bulkcopy_address_synonym"
236+
237+
# Create table
238+
cursor.execute(f"DROP TABLE IF EXISTS {table_name}")
239+
cursor.execute(f"""
240+
CREATE TABLE {table_name} (
241+
id INT,
242+
name NVARCHAR(50),
243+
value FLOAT
244+
)
245+
""")
246+
conn.commit()
247+
248+
# Test data
249+
test_data = [(1, "Test1", 1.5), (2, "Test2", 2.5), (3, "Test3", 3.5)]
250+
251+
# Perform bulkcopy with connection using Address parameter
252+
result = cursor.bulkcopy(table_name, test_data)
253+
254+
# Verify result
255+
assert result is not None
256+
assert "rows_copied" in result
257+
assert result["rows_copied"] == 3
258+
259+
# Verify data
260+
cursor.execute(f"SELECT COUNT(*) FROM {table_name}")
261+
count = cursor.fetchone()[0]
262+
assert count == 3
263+
264+
# Cleanup
265+
cursor.execute(f"DROP TABLE {table_name}")
266+
cursor.close()
267+
finally:
268+
conn.close()
269+
270+
# Test that bulkcopy fails when SERVER parameter is missing entirely
271+
params = parser._parse(conn_str)
272+
# Remove all server synonyms
273+
params.pop("server", None)
274+
params.pop("addr", None)
275+
params.pop("address", None)
276+
builder = _ConnectionStringBuilder(params)
277+
conn_string_no_server = builder.build()
278+
279+
# Ensure we have a valid connection string for the main connection
280+
conn = connect(conn_str)
281+
try:
282+
cursor = conn.cursor()
283+
# Manually override the connection string to one without server
284+
cursor.connection.connection_str = conn_string_no_server
285+
286+
table_name = "test_bulkcopy_no_server"
287+
test_data = [(1, "Test1", 1.5)]
288+
289+
# This should raise ValueError due to missing SERVER parameter
290+
try:
291+
cursor.bulkcopy(table_name, test_data)
292+
assert False, "Expected ValueError for missing SERVER parameter"
293+
except ValueError as e:
294+
assert "SERVER parameter is required" in str(e)
295+
296+
cursor.close()
297+
finally:
298+
conn.close()

0 commit comments

Comments
 (0)