turn off use_insertmanyvalues for SQL Server

we will keep trying to find workarounds, however this
patch is the "turn it off" patch

Due to a critical bug identified in SQL Server, the SQLAlchemy
"insertmanyvalues" feature which allows fast INSERT of many rows while also
supporting RETURNING unfortunately needs to be disabled for SQL Server. SQL
Server is apparently unable to guarantee that the order of rows inserted
matches the order in which they are sent back by OUTPUT inserted when
table-valued rows are used with INSERT in conjunction with OUTPUT inserted.
We are trying to see if Microsoft is able to confirm this undocumented
behavior however there is no known workaround, other than it's not safe to
use table-valued expressions with OUTPUT inserted for now.

Fixes: #9603
Change-Id: I4b932fb8774390bbdf4e870a1f6cfe9a78c4b105

3 files changed, 36 insertions, 16 deletions

diff --git a/doc/build/changelog/unreleased_20/9603.rst b/doc/build/changelog/unreleased_20/9603.rst
new file mode 100644
index 000000000..0af82ae1f
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/9603.rst
@@ -0,0 +1,14 @@
+.. change::
+    :tags: bug, mssql
+    :tickets: 9603
+
+    Due to a critical bug identified in SQL Server, the SQLAlchemy
+    "insertmanyvalues" feature which allows fast INSERT of many rows while also
+    supporting RETURNING unfortunately needs to be disabled for SQL Server. SQL
+    Server is apparently unable to guarantee that the order of rows inserted
+    matches the order in which they are sent back by OUTPUT inserted when
+    table-valued rows are used with INSERT in conjunction with OUTPUT inserted.
+    We are trying to see if Microsoft is able to confirm this undocumented
+    behavior however there is no known workaround, other than it's not safe to
+    use table-valued expressions with OUTPUT inserted for now.
+
diff --git a/doc/build/changelog/whatsnew_20.rst b/doc/build/changelog/whatsnew_20.rst
index 63fadc8a5..6b023bb48 100644
--- a/doc/build/changelog/whatsnew_20.rst
+++ b/doc/build/changelog/whatsnew_20.rst
@@ -859,7 +859,8 @@ Optimized ORM bulk insert now implemented for all backends other than MySQL
 The dramatic performance improvement introduced in the 1.4 series and described
 at :ref:`change_5263` has now been generalized to all included backends that
 support RETURNING, which is all backends other than MySQL: SQLite, MariaDB,
-PostgreSQL (all drivers), Oracle, and SQL Server. While the original feature
+PostgreSQL (all drivers), and Oracle; SQL Server has support but unfortunately
+had to be turned off due to an issue with SQL Server [#]_. While the original feature
 was most critical for the psycopg2 driver which otherwise had major performance
 issues when using ``cursor.executemany()``, the change is also critical for
 other PostgreSQL drivers such as asyncpg, as when using RETURNING,
@@ -894,16 +895,16 @@ With most databases now offering RETURNING (with the conspicuous exception of
 MySQL, given that MariaDB supports it), the new change generalizes the psycopg2
 "fast execution helper" approach to all dialects that support RETURNING, which
 now includes SQlite and MariaDB, and for which no other approach for
-"executemany plus RETURNING" is possible, which includes SQLite, MariaDB, all
-PG drivers, and SQL Server. The cx_Oracle and oracledb drivers used for Oracle
+"executemany plus RETURNING" is possible, which includes SQLite, MariaDB, and all
+PG drivers. The cx_Oracle and oracledb drivers used for Oracle
 support RETURNING with executemany natively, and this has also been implemented
-to provide equivalent performance improvements. With SQLite and MariaDB now
+to provide equivalent performance improvements.  With SQLite and MariaDB now
 offering RETURNING support, ORM use of ``cursor.lastrowid`` is nearly a thing
 of the past, with only MySQL still relying upon it.
 
 For INSERT statements that don't use RETURNING, traditional executemany()
-behavior is used for most backends, with the current exceptions of psycopg2
-and mssql+pyodbc, which both have very slow executemany() performance overall
+behavior is used for most backends, with the current exception of psycopg2,
+which has very slow executemany() performance overall
 and are still improved by the "insertmanyvalues" approach.
 
 Benchmarks
@@ -974,10 +975,18 @@ sqlite+pysqlite2 (memory)      6.204843                3.554856
 postgresql+asyncpg (network)   88.292285               4.561492
 postgresql+psycopg (network)   N/A (psycopg3)          4.861368
 oracle+cx_Oracle (network)     92.603953               4.809520
-mssql+pyodbc (network)         158.396667              4.825139
 mariadb+mysqldb (network)      71.705197               4.075377
 ============================   ====================    ====================
 
+.. mssql+pyodbc (network)      .. 158.396667           .. 4.825139
+
+
+.. note::
+
+   .. [#] The feature is disabled for SQL Server as of SQLAlchemy 2.0.9 due
+      to incompatibilities in how table-valued expressions are handled by
+      SQL Server.  See https://github.com/sqlalchemy/sqlalchemy/issues/9603
+
 Two additional drivers have no change in performance; the psycopg2 drivers,
 for which fast executemany was already implemented in SQLAlchemy 1.4,
 and MySQL, which continues to not offer RETURNING support:
diff --git a/doc/build/core/connections.rst b/doc/build/core/connections.rst
index 80156b418..726b5a501 100644
--- a/doc/build/core/connections.rst
+++ b/doc/build/core/connections.rst
@@ -1854,7 +1854,9 @@ as follows:
 
 * SQLite - supported for SQLite versions 3.35 and above
 * PostgreSQL - all supported Postgresql versions (9 and above)
-* SQL Server - all supported SQL Server versions
+* SQL Server - **disabled by default as of SQLAlchemy 2.0.9** - the SQL syntax
+  used has been shown to not be safe for RETURNING
+  (see https://github.com/sqlalchemy/sqlalchemy/issues/9603)
 * MariaDB - supported for MariaDB versions 10.5 and above
 * MySQL - no support, no RETURNING feature is present
 * Oracle - supports RETURNING with executemany using native cx_Oracle / OracleDB
@@ -1887,11 +1889,7 @@ The feature can also be disabled from being used implicitly for a particular
       )
 
 The reason one might want to disable RETURNING for a specific table is to
-work around backend-specific limitations.  For example, there is a known
-limitation of SQL Server that the ``OUTPUT inserted.<colname>`` feature
-may not work correctly for a table that has INSERT triggers established;
-such a table may need to include ``implicit_returning=False`` (see
-:ref:`mssql_triggers`).
+work around backend-specific limitations.
 
 .. _engine_insertmanyvalues_page_size:
 
@@ -1929,9 +1927,8 @@ varies by dialect and server version; the largest size is 32700 (chosen as a
 healthy distance away from PostgreSQL's limit of 32767 and SQLite's modern
 limit of 32766, while leaving room for additional parameters in the statement
 as well as for DBAPI quirkiness). Older versions of SQLite (prior to 3.32.0)
-will set this value to 999; SQL Server sets it to 2099.  MariaDB has no
-established limit however 32700 remains as a limiting factor for SQL message
-size.
+will set this value to 999. MariaDB has no established limit however 32700
+remains as a limiting factor for SQL message size.
 
 The value of the "batch size" can be affected :class:`_engine.Engine`
 wide via the :paramref:`_sa.create_engine.insertmanyvalues_page_size` parameter.