disable setinputsizes only for true DBAPI executemany

Fixed regression caused by the combination of :ticket:`8177`, re-enable
setinputsizes for SQL server unless fast_executemany + DBAPI executemany is
used for a statement, along with :ticket:`6047`, implement
"insertmanyvalues", which bypasses DBAPI executemany in place of a custom
DBAPI execute for INSERT statements. setinputsizes would incorrectly not be
used for a multiple parameter-set INSERT statement that used
"insertmanyvalues" if fast_executemany were turned on, as the check would
incorrectly assume this is a DBAPI executemany call.  The "regression"
would then be that the "insertmanyvalues" statement format is apparently
slightly more sensitive to multiple rows that don't use the same types
for each row, so in such a case setinputsizes is especially needed.

The fix repairs the fast_executemany check so that it only disables
setinputsizes if true DBAPI executemany is to be used.

Fixes: #8917
Change-Id: I78895606a99848d4f92ecf38ded92dc5d6d48c6f

diff --git a/doc/build/changelog/unreleased_20/8917.rst b/doc/build/changelog/unreleased_20/8917.rst
new file mode 100644 (file)
index 0000000..37bee0d
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/8917.rst
@@ -0,0 +1,18 @@
+.. change::
+    :tags: bug, mssql
+    :tickets: 8917
+
+    Fixed regression caused by the combination of :ticket:`8177`, re-enable
+    setinputsizes for SQL server unless fast_executemany + DBAPI executemany is
+    used for a statement, along with :ticket:`6047`, implement
+    "insertmanyvalues", which bypasses DBAPI executemany in place of a custom
+    DBAPI execute for INSERT statements. setinputsizes would incorrectly not be
+    used for a multiple parameter-set INSERT statement that used
+    "insertmanyvalues" if fast_executemany were turned on, as the check would
+    incorrectly assume this is a DBAPI executemany call.  The "regression"
+    would then be that the "insertmanyvalues" statement format is apparently
+    slightly more sensitive to multiple rows that don't use the same types
+    for each row, so in such a case setinputsizes is especially needed.
+
+    The fix repairs the fast_executemany check so that it only disables
+    setinputsizes if true DBAPI executemany is to be used.

diff --git a/lib/sqlalchemy/connectors/pyodbc.py b/lib/sqlalchemy/connectors/pyodbc.py
index f920860cd8c3ee51441b57be6bf437f63ebc5137..e8fbed18eb6e35ae4e0aa08a3fb3cdf26df3b737 100644 (file)
--- a/lib/sqlalchemy/connectors/pyodbc.py
+++ b/lib/sqlalchemy/connectors/pyodbc.py
@@ -209,7 +209,10 @@ class PyODBCConnector(Connector):
         # omit the setinputsizes calls for .executemany() with
         # fast_executemany=True
 
-        if context.executemany and self.fast_executemany:
+        if (
+            context.execute_style is interfaces.ExecuteStyle.EXECUTEMANY
+            and self.fast_executemany
+        ):
             return
 
         cursor.setinputsizes(

diff --git a/test/dialect/mssql/test_engine.py b/test/dialect/mssql/test_engine.py
index 6b895e3f1c60ad2dccd43dc8f274340ec1e9a7e7..57c6274e726a0363a31ef921556151014796e112 100644 (file)
--- a/test/dialect/mssql/test_engine.py
+++ b/test/dialect/mssql/test_engine.py
@@ -419,69 +419,55 @@ class FastExecutemanyTest(fixtures.TestBase):
 
             conn.execute(t.insert(), {"id": 200, "data": "data_200"})
 
-    @testing.fixture
-    def fe_engine(self, testing_engine):
-        def go(use_fastexecutemany, apply_setinputsizes_flag):
-            engine = testing_engine(
-                options={
-                    "fast_executemany": use_fastexecutemany,
-                    "use_setinputsizes": apply_setinputsizes_flag,
-                }
-            )
-            return engine
-
-        return go
-
-    @testing.combinations(
-        (
-            "setinputsizeshook",
-            True,
-        ),
-        (
-            "nosetinputsizeshook",
-            False,
-        ),
-        argnames="include_setinputsizes",
-        id_="ia",
-    )
-    @testing.combinations(
-        (
-            "setinputsizesflag",
-            True,
-        ),
-        (
-            "nosetinputsizesflag",
-            False,
-        ),
-        argnames="apply_setinputsizes_flag",
-        id_="ia",
-    )
-    @testing.combinations(
-        (
-            "fastexecutemany",
-            True,
-        ),
-        (
-            "nofastexecutemany",
-            False,
-        ),
-        argnames="use_fastexecutemany",
-        id_="ia",
-    )
-    def test_insert_floats(
+    @testing.variation("add_event", [True, False])
+    @testing.variation("setinputsizes", [True, False])
+    @testing.variation("fastexecutemany", [True, False])
+    @testing.variation("insertmanyvalues", [True, False])
+    @testing.variation("broken_types", [True, False])
+    def test_insert_typing(
         self,
         metadata,
-        fe_engine,
-        include_setinputsizes,
-        use_fastexecutemany,
-        apply_setinputsizes_flag,
+        testing_engine,
+        add_event,
+        fastexecutemany,
+        setinputsizes,
+        insertmanyvalues,
+        broken_types,
     ):
+        """tests for executemany + datatypes that are sensitive to
+        "setinputsizes"
+
+        Issues tested here include:
+
+        #6058 - turn off setinputsizes by default, since it breaks with
+                fast_executemany (version 1.4)
+
+        #8177 - turn setinputsizes back **on** by default, just skip it only
+                for cursor.executemany() calls when fast_executemany is set;
+                otherwise use it.  (version 2.0)
+
+        #8917 - oops, we added "insertmanyvalues" but forgot to adjust the
+                check in #8177 above to accommodate for this, so
+                setinputsizes was getting turned off for "insertmanyvalues"
+                if fast_executemany was still set
+
+        """
 
         # changes for issue #8177 have eliminated all current expected
         # failures, but we'll leave this here in case we need it again
-        expect_failure = False
+        # (... four months pass ...)
+        # surprise! we need it again.  woop!  for #8917
+        expect_failure = (
+            broken_types and not setinputsizes and insertmanyvalues
+        )
 
-        engine = fe_engine(use_fastexecutemany, apply_setinputsizes_flag)
+        engine = testing_engine(
+            options={
+                "fast_executemany": fastexecutemany,
+                "use_setinputsizes": setinputsizes,
+                "use_insertmanyvalues": insertmanyvalues,
+            }
+        )
 
         observations = Table(
             "Observations",
@@ -489,6 +475,7 @@ class FastExecutemanyTest(fixtures.TestBase):
             Column("id", Integer, nullable=False, primary_key=True),
             Column("obs1", Numeric(19, 15), nullable=True),
             Column("obs2", Numeric(19, 15), nullable=True),
+            Column("obs3", String(10)),
             schema="test_schema",
         )
         with engine.begin() as conn:
@@ -499,20 +486,33 @@ class FastExecutemanyTest(fixtures.TestBase):
                 "id": 1,
                 "obs1": Decimal("60.1722066045792"),
                 "obs2": Decimal("24.929289808227466"),
+                "obs3": "obs3",
             },
             {
                 "id": 2,
                 "obs1": Decimal("60.16325715615476"),
                 "obs2": Decimal("24.93886459535008"),
+                "obs3": 5 if broken_types else "obs3",
             },
             {
                 "id": 3,
                 "obs1": Decimal("60.16445165123469"),
                 "obs2": Decimal("24.949856300109516"),
+                "obs3": 7 if broken_types else "obs3",
             },
         ]
 
-        if include_setinputsizes:
+        assert_records = [
+            {
+                "id": rec["id"],
+                "obs1": rec["obs1"],
+                "obs2": rec["obs2"],
+                "obs3": str(rec["obs3"]),
+            }
+            for rec in records
+        ]
+
+        if add_event:
             canary = mock.Mock()
 
             @event.listens_for(engine, "do_setinputsizes")
@@ -543,16 +543,23 @@ class FastExecutemanyTest(fixtures.TestBase):
                     )
                     .mappings()
                     .all(),
-                    records,
+                    assert_records,
                 )
 
-        if include_setinputsizes:
-            if apply_setinputsizes_flag:
+        if add_event:
+            if setinputsizes:
                 eq_(
                     canary.mock_calls,
                     [
                         # float for int?  this seems wrong
-                        mock.call([float, float, float]),
+                        mock.call(
+                            [
+                                float,
+                                float,
+                                float,
+                                engine.dialect.dbapi.SQL_VARCHAR,
+                            ]
+                        ),
                         mock.call([]),
                     ],
                 )