Use _effective_decimal_return_scale in Numeric.result_processor

Fixed an issue in :class:`.Numeric` where the
:paramref:`.Numeric.decimal_return_scale` parameter was ignored when the
DBAPI does not support native decimal objects (i.e.
``dialect.supports_native_decimal`` is ``False``).  In this path the result
processor was computing the conversion scale from
:paramref:`.Numeric.scale` directly, bypassing
:paramref:`.Numeric.decimal_return_scale` entirely.  The behavior now
matches :class:`.Float`, which already used the correct
``_effective_decimal_return_scale`` property. Pull request courtesy Kadir
Can Ozden.

Fixes: #13424
Closes: #13137
Co-authored-by: Mike Bayer <mike_mp@zzzcomputing.com>
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/13137
Pull-request-sha: 9d5dbc80a4d053b46454c87d56d36b51c672a2dc

Change-Id: Ib671aa9db19c64fe69d95a47a4b8420b96e918b9

diff --git a/doc/build/changelog/unreleased_20/13424.rst b/doc/build/changelog/unreleased_20/13424.rst
new file mode 100644 (file)
index 0000000..970839d
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/13424.rst
@@ -0,0 +1,14 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 13424
+
+    Fixed an issue in :class:`.Numeric` where the
+    :paramref:`.Numeric.decimal_return_scale` parameter was ignored when the
+    DBAPI does not support native decimal objects (i.e.
+    ``dialect.supports_native_decimal`` is ``False``).  In this path the result
+    processor was computing the conversion scale from
+    :paramref:`.Numeric.scale` directly, bypassing
+    :paramref:`.Numeric.decimal_return_scale` entirely.  The behavior now
+    matches :class:`.Float`, which already used the correct
+    ``_effective_decimal_return_scale`` property. Pull request courtesy Kadir
+    Can Ozden.

diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py
index c5a11f5dbd8aecaf3dd0627e953867e0ed010a02..a3ce81da583c5b4fba42dcdf1b270371f722eb23 100644 (file)
--- a/lib/sqlalchemy/sql/sqltypes.py
+++ b/lib/sqlalchemy/sql/sqltypes.py
@@ -656,11 +656,7 @@ class Numeric(NumericCommon[_N], TypeEngine[_N]):
                 # we're a "numeric", DBAPI returns floats, convert.
                 return processors.to_decimal_processor_factory(
                     decimal.Decimal,
-                    (
-                        self.scale
-                        if self.scale is not None
-                        else self._default_decimal_return_scale
-                    ),
+                    self._effective_decimal_return_scale,
                 )
         else:
             if dialect.supports_native_decimal:

diff --git a/test/sql/test_types.py b/test/sql/test_types.py
index e927578a5a46a0327c30a26330f93f5c46422597..fd82a34ea0423907bf71adc6fee210903ebd028c 100644 (file)
--- a/test/sql/test_types.py
+++ b/test/sql/test_types.py
@@ -4199,6 +4199,105 @@ class NumericRawSQLTest(fixtures.TestBase):
         eq_(val, 46.583)
 
 
+class NumericDecimalReturnScaleTest(fixtures.TestBase):
+    """Test that Numeric and Float honour decimal_return_scale when the DBAPI
+    does not return native Decimal objects (supports_native_decimal=False).
+
+    issue #13424
+
+    """
+
+    # TODO: re-add when supports_native_decimal is fixed
+    # __sparse_driver_backend__ = True
+
+    numeric_cases = testing.combinations(
+        # Numeric: decimal_return_scale overrides scale
+        (
+            Numeric(10, 2, decimal_return_scale=5),
+            1.23456789,
+            decimal.Decimal("1.23457"),
+        ),
+        # Numeric: falls back to scale when decimal_return_scale not set
+        (Numeric(10, 2), 1.23456789, decimal.Decimal("1.23")),
+        # Numeric: falls back to _default_decimal_return_scale (10)
+        # when neither scale nor decimal_return_scale is set
+        (
+            Numeric(asdecimal=True),
+            1.23456789012345,
+            decimal.Decimal("1.2345678901"),
+        ),
+        # Float: decimal_return_scale (pre-existing behavior, for contrast);
+        # cases 1 and 4 returning the same value asserts consistency
+        (
+            Float(decimal_return_scale=5, asdecimal=True),
+            1.23456789,
+            decimal.Decimal("1.23457"),
+        ),
+        (
+            Float(asdecimal=True),
+            1.23456789012345,
+            decimal.Decimal("1.2345678901"),
+        ),
+        id_="nnn",
+        argnames="typ,value,expected",
+    )
+
+    @numeric_cases
+    def test_result_processor_decimal_return_scale(self, typ, value, expected):
+        """result_processor honours decimal_return_scale."""
+        proc = typ.result_processor(
+            mock.Mock(supports_native_decimal=False), None
+        )
+        eq_(proc(value), expected)
+
+    # TODO: make this a more general requirement, however for the moment
+    # the supports_native_decimal flag seems to not be correctly set for
+    # some dialects such as psycopg
+    @testing.only_on("sqlite")
+    @testing.combinations(
+        (
+            Numeric(10, 5, decimal_return_scale=3),
+            decimal.Decimal("1.12345"),
+            decimal.Decimal("1.123"),
+        ),
+        (
+            Float(decimal_return_scale=3, asdecimal=True),
+            1.12345,
+            decimal.Decimal("1.123"),
+        ),
+        id_="nnn",
+        argnames="typ,value,expected",
+    )
+    def test_decimal_return_scale_roundtrip(
+        self, connection, metadata, typ, value, expected
+    ):
+        """Numeric and Float both honour decimal_return_scale
+        end-to-end.
+
+        Only runs on SQLite where the DBAPI truly returns floats for
+        NUMERIC/FLOAT columns.  Other backends like psycopg return
+        native Decimal objects despite supports_native_decimal=False,
+        which bypasses the result processor being tested here.
+        """
+
+        t = Table(
+            "t",
+            metadata,
+            Column(
+                "numeric_val",
+                typ,
+            ),
+        )
+        t.create(connection)
+
+        connection.execute(
+            t.insert(),
+            {"numeric_val": value},
+        )
+
+        eq_(connection.scalar(select(t.c.numeric_val)), expected)
+
+
 class IntervalTest(fixtures.TablesTest, AssertsExecutionResults):
     __sparse_driver_backend__ = True