Skip to content

Commit b38fb59

Browse files
zzzeekzzzeek
committed
audition pymssql once more; retire sane_rowcount_returning
pymssql seems to be maintained again and seems to be working completely, so let's try re-enabling it. Fixed issue in the new :class:`.Uuid` datatype which prevented it from working with the pymssql driver. As pymssql seems to be maintained again, restored testing support for pymssql. Tweaked the pymssql dialect to take better advantage of RETURNING for INSERT statements in order to retrieve last inserted primary key values, in the same way as occurs for the mssql+pyodbc dialect right now. Identified that the ``sqlite`` and ``mssql+pyodbc`` dialects are now compatible with the SQLAlchemy ORM's "versioned rows" feature, since SQLAlchemy now computes rowcount for a RETURNING statement in this specific case by counting the rows returned, rather than relying upon ``cursor.rowcount``. In particular, the ORM versioned rows use case (documented at :ref:`mapper_version_counter`) should now be fully supported with the SQL Server pyodbc dialect. Change-Id: I38a0666587212327aecf8f98e86031ab25d1f14d References: sqlalchemy#5321 Fixes: sqlalchemy#9414
1 parent 7099dd2 commit b38fb59

18 files changed

Lines changed: 287 additions & 75 deletions

File tree

‎doc/build/changelog/unreleased_20/9414.rst‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
.. change::
2+
:tags: bug, mssql
3+
:tickets: 9414
4+
5+
Fixed issue in the new :class:`.Uuid` datatype which prevented it from
6+
working with the pymssql driver. As pymssql seems to be maintained again,
7+
restored testing support for pymssql.
8+
9+
.. change::
10+
:tags: bug, mssql
11+
12+
Tweaked the pymssql dialect to take better advantage of
13+
RETURNING for INSERT statements in order to retrieve last inserted primary
14+
key values, in the same way as occurs for the mssql+pyodbc dialect right
15+
now.
16+
17+
.. change::
18+
:tags: bug, orm
19+
20+
Identified that the ``sqlite`` and ``mssql+pyodbc`` dialects are now
21+
compatible with the SQLAlchemy ORM's "versioned rows" feature, since
22+
SQLAlchemy now computes rowcount for a RETURNING statement in this specific
23+
case by counting the rows returned, rather than relying upon
24+
``cursor.rowcount``. In particular, the ORM versioned rows use case
25+
(documented at :ref:`mapper_version_counter`) should now be fully
26+
supported with the SQL Server pyodbc dialect.
27+

‎lib/sqlalchemy/dialects/mssql/base.py‎

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -885,29 +885,20 @@ class MyClass(Base):
885885
of rows updated from an UPDATE or DELETE statement.
886886
887887
As of this writing, the PyODBC driver is not able to return a rowcount when
888-
OUTPUT INSERTED is used. This impacts the SQLAlchemy ORM's versioning feature
889-
in many cases where server-side value generators are in use in that while the
890-
versioning operations can succeed, the ORM cannot always check that an UPDATE
891-
or DELETE statement matched the number of rows expected, which is how it
892-
verifies that the version identifier matched. When this condition occurs, a
893-
warning will be emitted but the operation will proceed.
894-
895-
The use of OUTPUT INSERTED can be disabled by setting the
896-
:paramref:`_schema.Table.implicit_returning` flag to ``False`` on a particular
897-
:class:`_schema.Table`, which in declarative looks like::
898-
899-
class MyTable(Base):
900-
__tablename__ = 'mytable'
901-
id = Column(Integer, primary_key=True)
902-
stuff = Column(String(10))
903-
timestamp = Column(TIMESTAMP(), default=text('DEFAULT'))
904-
__mapper_args__ = {
905-
'version_id_col': timestamp,
906-
'version_id_generator': False,
907-
}
908-
__table_args__ = {
909-
'implicit_returning': False
910-
}
888+
OUTPUT INSERTED is used. Previous versions of SQLAlchemy therefore had
889+
limitations for features such as the "ORM Versioning" feature that relies upon
890+
accurate rowcounts in order to match version numbers with matched rows.
891+
892+
SQLAlchemy 2.0 now retrieves the "rowcount" manually for these particular use
893+
cases based on counting the rows that arrived back within RETURNING; so while
894+
the driver still has this limitation, the ORM Versioning feature is no longer
895+
impacted by it. As of SQLAlchemy 2.0.5, ORM versioning has been fully
896+
re-enabled for the pyodbc driver.
897+
898+
.. versionchanged:: 2.0.5 ORM versioning support is restored for the pyodbc
899+
driver. Previously, a warning would be emitted during ORM flush that
900+
versioning was not supported.
901+
911902
912903
Enabling Snapshot Isolation
913904
---------------------------
@@ -2965,6 +2956,7 @@ class MSDialect(default.DefaultDialect):
29652956
supports_statement_cache = True
29662957
supports_default_values = True
29672958
supports_empty_insert = False
2959+
favor_returning_over_lastrowid = True
29682960

29692961
supports_comments = True
29702962
supports_default_metavalue = False

‎lib/sqlalchemy/dialects/mssql/provision.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,35 @@
1212
from ...testing.provision import create_db
1313
from ...testing.provision import drop_all_schema_objects_pre_tables
1414
from ...testing.provision import drop_db
15+
from ...testing.provision import generate_driver_url
1516
from ...testing.provision import get_temp_table_name
1617
from ...testing.provision import log
1718
from ...testing.provision import normalize_sequence
1819
from ...testing.provision import run_reap_dbs
1920
from ...testing.provision import temp_table_keyword_args
2021

2122

23+
@generate_driver_url.for_db("mssql")
24+
def generate_driver_url(url, driver, query_str):
25+
26+
backend = url.get_backend_name()
27+
28+
new_url = url.set(drivername="%s+%s" % (backend, driver))
29+
30+
if driver != "pyodbc":
31+
new_url = new_url.set(query="")
32+
33+
if query_str:
34+
new_url = new_url.update_query_string(query_str)
35+
36+
try:
37+
new_url.get_dialect()
38+
except exc.NoSuchModuleError:
39+
return None
40+
else:
41+
return new_url
42+
43+
2244
@create_db.for_db("mssql")
2345
def _mssql_create_db(cfg, eng, ident):
2446
with eng.connect().execution_options(isolation_level="AUTOCOMMIT") as conn:

‎lib/sqlalchemy/dialects/mssql/pymssql.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,9 @@
1616
pymssql is a Python module that provides a Python DBAPI interface around
1717
`FreeTDS <https://www.freetds.org/>`_.
1818
19-
.. note::
19+
.. versionchanged:: 2.0.5
2020
21-
pymssql is currently not included in SQLAlchemy's continuous integration
22-
(CI) testing.
21+
pymssql was restored to SQLAlchemy's continuous integration testing
2322
2423
2524
""" # noqa
@@ -51,6 +50,7 @@ def __init__(self, dialect):
5150
class MSDialect_pymssql(MSDialect):
5251
supports_statement_cache = True
5352
supports_native_decimal = True
53+
supports_native_uuid = True
5454
driver = "pymssql"
5555

5656
preparer = MSIdentifierPreparer_pymssql

‎lib/sqlalchemy/dialects/mssql/pyodbc.py‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -281,9 +281,9 @@ def provide_token(dialect, conn_rec, cargs, cparams):
281281
Rowcount Support
282282
----------------
283283
284-
Pyodbc only has partial support for rowcount. See the notes at
285-
:ref:`mssql_rowcount_versioning` for important notes when using ORM
286-
versioning.
284+
Previous limitations with the SQLAlchemy ORM's "versioned rows" feature with
285+
Pyodbc have been resolved as of SQLAlchemy 2.0.5. See the notes at
286+
:ref:`mssql_rowcount_versioning`.
287287
288288
.. _mssql_pyodbc_fastexecutemany:
289289
@@ -607,11 +607,10 @@ def post_exec(self):
607607
class MSDialect_pyodbc(PyODBCConnector, MSDialect):
608608
supports_statement_cache = True
609609

610-
# mssql still has problems with this on Linux
610+
# note this parameter is no longer used by the ORM or default dialect
611+
# see #9414
611612
supports_sane_rowcount_returning = False
612613

613-
favor_returning_over_lastrowid = True
614-
615614
execution_ctx_cls = MSExecutionContext_pyodbc
616615

617616
colspecs = util.update_copy(

‎lib/sqlalchemy/dialects/sqlite/base.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1910,7 +1910,10 @@ class SQLiteDialect(default.DefaultDialect):
19101910
supports_default_values = True
19111911
supports_default_metavalue = False
19121912

1913+
# sqlite issue:
19131914
# https://github.com/python/cpython/issues/93421
1915+
# note this parameter is no longer used by the ORM or default dialect
1916+
# see #9414
19141917
supports_sane_rowcount_returning = False
19151918

19161919
supports_empty_insert = False

‎lib/sqlalchemy/orm/persistence.py‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -805,11 +805,7 @@ def update_stmt(existing_stmt=None):
805805
statement = statement.return_defaults(mapper.version_id_col)
806806
return_defaults = True
807807

808-
assert_singlerow = (
809-
connection.dialect.supports_sane_rowcount
810-
if not return_defaults
811-
else connection.dialect.supports_sane_rowcount_returning
812-
)
808+
assert_singlerow = connection.dialect.supports_sane_rowcount
813809

814810
assert_multirow = (
815811
assert_singlerow
@@ -1274,9 +1270,6 @@ def update_stmt():
12741270

12751271
if mapper._version_id_has_server_side_value:
12761272
statement = statement.return_defaults(mapper.version_id_col)
1277-
return_defaults = True
1278-
else:
1279-
return_defaults = False
12801273

12811274
# execute each UPDATE in the order according to the original
12821275
# list of states to guarantee row access order, but
@@ -1291,11 +1284,7 @@ def update_stmt():
12911284
records = list(records)
12921285
connection = key[0]
12931286

1294-
assert_singlerow = (
1295-
connection.dialect.supports_sane_rowcount
1296-
if not return_defaults
1297-
else connection.dialect.supports_sane_rowcount_returning
1298-
)
1287+
assert_singlerow = connection.dialect.supports_sane_rowcount
12991288
assert_multirow = (
13001289
assert_singlerow
13011290
and connection.dialect.supports_sane_multi_rowcount

‎lib/sqlalchemy/testing/requirements.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,13 @@ def implements_get_lastrowid(self):
492492
"""
493493
return exclusions.open()
494494

495+
@property
496+
def arraysize(self):
497+
"""dialect includes the required pep-249 attribute
498+
``cursor.arraysize``"""
499+
500+
return exclusions.open()
501+
495502
@property
496503
def emulated_lastrowid(self):
497504
"""target dialect retrieves cursor.lastrowid, or fetches
@@ -850,6 +857,24 @@ def time_timezone(self):
850857

851858
return exclusions.closed()
852859

860+
@property
861+
def date_implicit_bound(self):
862+
"""target dialect when given a date object will bind it such
863+
that the database server knows the object is a date, and not
864+
a plain string.
865+
866+
"""
867+
return exclusions.open()
868+
869+
@property
870+
def time_implicit_bound(self):
871+
"""target dialect when given a time object will bind it such
872+
that the database server knows the object is a time, and not
873+
a plain string.
874+
875+
"""
876+
return exclusions.open()
877+
853878
@property
854879
def datetime_implicit_bound(self):
855880
"""target dialect when given a datetime object will bind it such

‎lib/sqlalchemy/testing/suite/test_rowcount.py‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from sqlalchemy import bindparam
44
from sqlalchemy import Column
55
from sqlalchemy import Integer
6+
from sqlalchemy import MetaData
67
from sqlalchemy import select
78
from sqlalchemy import String
89
from sqlalchemy import Table
@@ -88,23 +89,58 @@ def test_update_rowcount2(self, connection):
8889
)
8990
eq_(r.rowcount, 3)
9091

91-
@testing.requires.update_returning
92-
def test_update_rowcount_return_defaults(self, connection):
92+
@testing.variation("implicit_returning", [True, False])
93+
@testing.variation(
94+
"dml",
95+
[
96+
("update", testing.requires.update_returning),
97+
("delete", testing.requires.delete_returning),
98+
],
99+
)
100+
def test_update_delete_rowcount_return_defaults(
101+
self, connection, implicit_returning, dml
102+
):
93103
"""note this test should succeed for all RETURNING backends
94104
as of 2.0. In
95105
Idf28379f8705e403a3c6a937f6a798a042ef2540 we changed rowcount to use
96106
len(rows) when we have implicit returning
97107
98108
"""
99-
employees_table = self.tables.employees
109+
110+
if implicit_returning:
111+
employees_table = self.tables.employees
112+
else:
113+
employees_table = Table(
114+
"employees",
115+
MetaData(),
116+
Column(
117+
"employee_id",
118+
Integer,
119+
autoincrement=False,
120+
primary_key=True,
121+
),
122+
Column("name", String(50)),
123+
Column("department", String(1)),
124+
implicit_returning=False,
125+
)
100126

101127
department = employees_table.c.department
102-
stmt = (
103-
employees_table.update()
104-
.where(department == "C")
105-
.values(name=employees_table.c.department + "Z")
106-
.return_defaults()
107-
)
128+
129+
if dml.update:
130+
stmt = (
131+
employees_table.update()
132+
.where(department == "C")
133+
.values(name=employees_table.c.department + "Z")
134+
.return_defaults()
135+
)
136+
elif dml.delete:
137+
stmt = (
138+
employees_table.delete()
139+
.where(department == "C")
140+
.return_defaults()
141+
)
142+
else:
143+
dml.fail()
108144

109145
r = connection.execute(stmt)
110146
eq_(r.rowcount, 3)

0 commit comments

Comments
 (0)