Problem
set_db_settings in datafaker/utils.py toggles connection.autocommit directly on the raw DBAPI connection:
existing_autocommit = connection.autocommit
connection.autocommit = True
# ... cursor.execute(sql) ...
connection.autocommit = existing_autocommit
autocommit is a DBAPI extension, not required by DB-API 2.0. Drivers differ in whether they expose it, and some builds of pymssql raise AttributeError when it is read or written.
Scope — DuckDB only
set_db_settings is only ever called for DuckDB connections. The connect-event listener that invokes it is registered only when parquet_dir is not None, which is a DuckDB source feature. PostgreSQL and MS-SQL connections never reach this code path, so the bug is latent — no user can hit it today.
Decision — skip for now
Two reasons to defer:
-
Incomplete fix. Even if the autocommit toggle were made defensive, the SET {k} TO {v} SQL syntax emitted by the function is DuckDB/PostgreSQL-specific. MS-SQL uses SET {k} {v} (no TO). Fixing autocommit in isolation still leaves the SQL broken for any MS-SQL caller, so it does not unblock anything.
-
No active users. The function is DuckDB-only. Fixing a latent risk speculatively, while leaving a concrete syntax problem open, is poor value for the effort.
The right time to address this is alongside a concrete requirement to call set_db_settings (or an equivalent) against an MS-SQL connection. At that point both the autocommit guard and the SQL syntax should be handled together.
Problem
set_db_settingsindatafaker/utils.pytogglesconnection.autocommitdirectly on the raw DBAPI connection:autocommitis a DBAPI extension, not required by DB-API 2.0. Drivers differ in whether they expose it, and some builds ofpymssqlraiseAttributeErrorwhen it is read or written.Scope — DuckDB only
set_db_settingsis only ever called for DuckDB connections. The connect-event listener that invokes it is registered only whenparquet_dir is not None, which is a DuckDB source feature. PostgreSQL and MS-SQL connections never reach this code path, so the bug is latent — no user can hit it today.Decision — skip for now
Two reasons to defer:
Incomplete fix. Even if the
autocommittoggle were made defensive, theSET {k} TO {v}SQL syntax emitted by the function is DuckDB/PostgreSQL-specific. MS-SQL usesSET {k} {v}(noTO). Fixing autocommit in isolation still leaves the SQL broken for any MS-SQL caller, so it does not unblock anything.No active users. The function is DuckDB-only. Fixing a latent risk speculatively, while leaving a concrete syntax problem open, is poor value for the effort.
The right time to address this is alongside a concrete requirement to call
set_db_settings(or an equivalent) against an MS-SQL connection. At that point both theautocommitguard and the SQL syntax should be handled together.