From 8c170bf9cd26e651f7844ed14ee5aac853bb6cd1 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" <3815206+peco-engineer-bot[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:41:40 +0000 Subject: [PATCH 01/14] [coverage] Conformance findings: PARAMQUERY-021 (#901) Signed-off-by: peco-engineer-bot[bot] <3815206+peco-engineer-bot[bot]@users.noreply.github.com> --- src/databricks/sql/parameters/native.py | 35 ++++++++++++++++++++++--- tests/e2e/test_parameterized_queries.py | 20 ++++++++++++++ tests/unit/test_parameters.py | 13 +++++++++ 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index d0d811940..3f01f1da6 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -615,9 +615,10 @@ def __init__( The `name` argument to this function would be `my_param`. :scale: - The maximum precision (total number of digits) of the number between 1 and 38. + The number of digits to the right of the decimal point. If the value itself + has more fractional digits, the value's scale is used so no digits are lost. :precision: - The number of digits to the right of the decimal point. + The maximum precision (total number of digits) of the number between 1 and 38. """ self.value: decimal.Decimal = value self.name = name @@ -638,11 +639,37 @@ def valid_scale_and_precision(self): return False def _cast_expr(self): - if self.scale and self.precision: - return self.CAST_EXPR.format(self.scale, self.precision) + if self.scale is not None and self.precision is not None: + precision, scale = self._effective_precision_and_scale(self.precision, self.scale) + # SQL DECIMAL type syntax is DECIMAL(precision, scale) + return self.CAST_EXPR.format(precision, scale) else: return self.calculate_decimal_cast_string(self.value) + MAX_PRECISION = 38 + + def _effective_precision_and_scale(self, precision: int, scale: int): + """Widen a declared DECIMAL(precision, scale) so the bound value never loses + fractional digits: the value's own scale is authoritative over an + under-declared target scale. Integer-digit room from the declared type is kept. + """ + try: + sign, digits, exponent = self.value.as_tuple() + except AttributeError: + return precision, scale + if not isinstance(exponent, int): + # NaN / Infinity: leave the declared type as-is + return precision, scale + + value_scale = max(-exponent, 0) + value_int_digits = max(len(digits) + exponent, 0) + + effective_scale = max(scale, value_scale) + int_digits = max(precision - scale, value_int_digits) + effective_precision = min(int_digits + effective_scale, self.MAX_PRECISION) + effective_scale = min(effective_scale, effective_precision) + return effective_precision, effective_scale + def calculate_decimal_cast_string(self, input: decimal.Decimal) -> str: """Returns the smallest SQL cast argument that can contain the passed decimal diff --git a/tests/e2e/test_parameterized_queries.py b/tests/e2e/test_parameterized_queries.py index 7370eea93..67ffd574d 100644 --- a/tests/e2e/test_parameterized_queries.py +++ b/tests/e2e/test_parameterized_queries.py @@ -510,6 +510,26 @@ def test_native_recursive_complex_type( ) assert self._recursive_compare(result.col, data) + @pytest.mark.parametrize("extra_params", [{}, {"use_sea": True}]) + def test_decimal_explicit_precision_scale_order(self, extra_params): + """PARAMQUERY-021 (a): a declared DECIMAL(10,2) target (precision=10, scale=2) + must be sent as DECIMAL(10,2), not DECIMAL(2,10), and the value round-trips.""" + param = DecimalParameter(Decimal("123.45"), scale=2, precision=10) + with self.cursor(extra_params) as cursor: + result = cursor.execute("SELECT ? AS v", [param]).fetchone() + + assert result.v == Decimal("123.45") + + @pytest.mark.parametrize("extra_params", [{}, {"use_sea": True}]) + def test_decimal_target_type_never_truncates_bound_scale(self, extra_params): + """PARAMQUERY-021 (c): the value's own scale (4) is authoritative over an + under-declared target scale (2): all four fractional digits must survive.""" + param = DecimalParameter(Decimal("123.4567"), scale=2, precision=10) + with self.cursor(extra_params) as cursor: + result = cursor.execute("SELECT ? AS v", [param]).fetchone() + + assert result.v == Decimal("123.4567") + class TestInlineParameterSyntax(PySQLPytestTestCase): """The inline parameter approach uses pyformat markers""" diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index 0588eb499..9b53050a3 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -107,6 +107,19 @@ def test_calculate_decimal_cast_string(value, expected): assert p._cast_expr() == expected +@pytest.mark.parametrize( + "value,scale,precision,expected", + ( + (Decimal("123.45"), 2, 10, "DECIMAL(10,2)"), + (Decimal("123"), 0, 10, "DECIMAL(10,0)"), + ), +) +def test_decimal_explicit_precision_scale_cast_string(value, scale, precision, expected): + """PARAMQUERY-021: SQL DECIMAL type is DECIMAL(precision, scale).""" + p = DecimalParameter(value, scale=scale, precision=precision) + assert p._cast_expr() == expected + + class Primitive(Enum): """These are the inferrable types. This Enum is used for parametrized tests.""" From 5e09830ce10af36cc2905d73e2c3a189b6946b75 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 21:49:50 +0000 Subject: [PATCH 02/14] ai: apply changes for #1006 (3 review threads) Addresses: - #4418237463 at src/databricks/sql/parameters/native.py:642 - #4418237473 at src/databricks/sql/parameters/native.py:650 - #4418237477 at tests/unit/test_parameters.py:113 Signed-off-by: peco-engineer-bot[bot] --- CHANGELOG.md | 1 + examples/parameters.py | 8 +++++--- src/databricks/sql/parameters/native.py | 22 +++++++++++++++++++++- tests/unit/test_parameters.py | 21 +++++++++++++++++++++ 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 219de41c0..e3070f0d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Release History # Unreleased +- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Passing `scale > precision` now raises `ValueError`, and a declared `scale` smaller than the value's own scale is widened so fractional digits are not lost. - Preserve the originating session when re-fetching or concurrently polling async statements on the kernel backend. # 4.8.0 (2026-10-24) diff --git a/examples/parameters.py b/examples/parameters.py index 93136ec71..21d1941d2 100644 --- a/examples/parameters.py +++ b/examples/parameters.py @@ -107,9 +107,11 @@ # will infer the Decimal to be a DECIMAL and will preserve its current precision and scale. ex4_p3 = Decimal("12.3456") -# For this parameter value, we bind a decimal with custom scale and precision -# that will result in the decimal being truncated. -ex4_p4 = DecimalParameter(value=ex4_p3, scale=4, precision=2) +# For this parameter value, we bind a decimal with custom precision and scale, +# i.e. DECIMAL(precision, scale). The declared scale is a minimum: if the value has +# more fractional digits, the bound type is widened so no digits are lost. Here +# DECIMAL(10,2) is widened to DECIMAL(12,4) and 12.3456 round-trips unchanged. +ex4_p4 = DecimalParameter(value=ex4_p3, precision=10, scale=2) EX4_QUERY = "SELECT ? `p1`, ? `p2`, ? `p3`, ? `p4`" EX4_PARAMS = [ex4_p1, ex4_p2, ex4_p3, ex4_p4] diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index 3f01f1da6..75f5a030b 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -600,6 +600,12 @@ def __init__( If set, `scale` and `precision` must both be set. If neither is set, the value will be casted to the smallest possible DECIMAL type that can contain it. + When set, `precision` and `scale` declare a *minimum* DECIMAL(precision, scale) + type: if the value needs more fractional or integer digits than declared, the + bound type is widened to fit it (capped at precision 38) so no digits are lost. + The value is never rounded or truncated to a narrower declared type; round the + `Decimal` yourself (e.g. with `Decimal.quantize`) before binding if you need that. + :value: The value to bind for this parameter. This will be casted to a DECIMAL. :name: @@ -618,7 +624,9 @@ def __init__( The number of digits to the right of the decimal point. If the value itself has more fractional digits, the value's scale is used so no digits are lost. :precision: - The maximum precision (total number of digits) of the number between 1 and 38. + The minimum precision (total number of digits) of the number, between 1 and 38. + If the value has more integer digits than `precision - scale` allows, or its + scale is widened, the precision grows to fit it (up to 38). """ self.value: decimal.Decimal = value self.name = name @@ -630,6 +638,18 @@ def __init__( "DecimalParameter requires both or none of scale and precision to be set" ) + if self.scale is not None and self.precision is not None and self.scale > self.precision: + # Earlier releases documented these arguments the other way round + # (scale = total digits, precision = fractional digits). Fail loudly + # instead of silently reinterpreting a call written that way. + raise ValueError( + "DecimalParameter scale ({}) must not exceed precision ({}). `precision` is " + "the total number of digits and `scale` is the number of digits after the " + "decimal point, as in SQL DECIMAL(precision, scale).".format( + self.scale, self.precision + ) + ) + def valid_scale_and_precision(self): if (self.scale is None and self.precision is None) or ( isinstance(self.scale, int) and isinstance(self.precision, int) diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index 9b53050a3..a3e78b137 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -112,6 +112,21 @@ def test_calculate_decimal_cast_string(value, expected): ( (Decimal("123.45"), 2, 10, "DECIMAL(10,2)"), (Decimal("123"), 0, 10, "DECIMAL(10,0)"), + # Value scale exceeds declared scale: scale widens, declared int room (8) is kept + (Decimal("123.4567"), 2, 10, "DECIMAL(12,4)"), + # Value has more integer digits than declared precision - scale allows + (Decimal("123456789012.5"), 2, 10, "DECIMAL(14,2)"), + # Positive exponent: 1E+3 needs 4 integer digits, no fractional digits + (Decimal("1E+3"), 0, 2, "DECIMAL(4,0)"), + # Widened precision is capped at 38 + (Decimal("0.123456789012345678901234567890123456"), 2, 10, "DECIMAL(38,36)"), + # Widened scale is capped at the (capped) precision + (Decimal("0.1234567890123456789012345678901234567890"), 2, 10, "DECIMAL(38,38)"), + # NaN / Infinity leave the declared type untouched + (Decimal("NaN"), 2, 10, "DECIMAL(10,2)"), + (Decimal("Infinity"), 2, 10, "DECIMAL(10,2)"), + # Non-Decimal values (no as_tuple) fall back to the declared type + (1.5, 2, 10, "DECIMAL(10,2)"), ), ) def test_decimal_explicit_precision_scale_cast_string(value, scale, precision, expected): @@ -120,6 +135,12 @@ def test_decimal_explicit_precision_scale_cast_string(value, scale, precision, e assert p._cast_expr() == expected +def test_decimal_scale_greater_than_precision_rejected(): + """Callers using the old (swapped) argument convention must fail loudly.""" + with pytest.raises(ValueError, match="must not exceed precision"): + DecimalParameter(Decimal("12.3456"), scale=4, precision=2) + + class Primitive(Enum): """These are the inferrable types. This Enum is used for parametrized tests.""" From d578b1713718cbf4fc23edc303f0317af6f7dfcc Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 21:56:07 +0000 Subject: [PATCH 03/14] ai: apply changes for #1006 (1 review thread) Addresses: - #4418299211 at src/databricks/sql/parameters/native.py:641 Signed-off-by: peco-engineer-bot[bot] --- CHANGELOG.md | 2 +- src/databricks/sql/parameters/native.py | 23 +++++++++++++++-------- tests/unit/test_parameters.py | 11 +++++++---- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3070f0d1..b6920a6cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Release History # Unreleased -- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Passing `scale > precision` now raises `ValueError`, and a declared `scale` smaller than the value's own scale is widened so fractional digits are not lost. +- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `DeprecationWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened so fractional digits are not lost. - Preserve the originating session when re-fetching or concurrently polling async statements on the kernel backend. # 4.8.0 (2026-10-24) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index 75f5a030b..cb636ce27 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -2,6 +2,7 @@ import datetime import decimal +import warnings from enum import Enum, auto from typing import Optional, Sequence, Any, TYPE_CHECKING @@ -640,15 +641,21 @@ def __init__( if self.scale is not None and self.precision is not None and self.scale > self.precision: # Earlier releases documented these arguments the other way round - # (scale = total digits, precision = fractional digits). Fail loudly - # instead of silently reinterpreting a call written that way. - raise ValueError( - "DecimalParameter scale ({}) must not exceed precision ({}). `precision` is " - "the total number of digits and `scale` is the number of digits after the " - "decimal point, as in SQL DECIMAL(precision, scale).".format( - self.scale, self.precision - ) + # (scale = total digits, precision = fractional digits). A valid + # DECIMAL can never have scale > precision, so this can only be a call + # written for the old convention: keep it working for now (swap the + # arguments) but warn so callers migrate before this becomes an error. + warnings.warn( + "DecimalParameter received scale ({}) > precision ({}); interpreting the " + "arguments using the legacy (swapped) convention as DECIMAL({},{}). " + "`precision` is the total number of digits and `scale` is the number of " + "digits after the decimal point, as in SQL DECIMAL(precision, scale). " + "Swap your arguments; this will raise ValueError in a future " + "release.".format(self.scale, self.precision, self.scale, self.precision), + DeprecationWarning, + stacklevel=2, ) + self.scale, self.precision = self.precision, self.scale def valid_scale_and_precision(self): if (self.scale is None and self.precision is None) or ( diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index a3e78b137..ab4d1bc08 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -135,10 +135,13 @@ def test_decimal_explicit_precision_scale_cast_string(value, scale, precision, e assert p._cast_expr() == expected -def test_decimal_scale_greater_than_precision_rejected(): - """Callers using the old (swapped) argument convention must fail loudly.""" - with pytest.raises(ValueError, match="must not exceed precision"): - DecimalParameter(Decimal("12.3456"), scale=4, precision=2) +def test_decimal_scale_greater_than_precision_legacy_swapped(): + """Callers using the old (swapped) convention keep working, with a deprecation warning.""" + with pytest.warns(DeprecationWarning, match="legacy"): + p = DecimalParameter(Decimal("123.45"), scale=10, precision=2) + assert p.precision == 10 + assert p.scale == 2 + assert p._cast_expr() == "DECIMAL(10,2)" class Primitive(Enum): From 72cdd600fc9828f9dcfa30a3a553f24596a2cc3f Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:02:49 +0000 Subject: [PATCH 04/14] ai: apply changes for #1006 (3 review threads) Addresses: - #4418346548 at src/databricks/sql/parameters/native.py:696 - #4418346556 at src/databricks/sql/parameters/native.py:697 - #4418346563 at src/databricks/sql/parameters/native.py:655 Signed-off-by: peco-engineer-bot[bot] --- CHANGELOG.md | 2 +- src/databricks/sql/parameters/native.py | 35 +++++++++++++++++++------ tests/unit/test_parameters.py | 7 ++++- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6920a6cb..1d55ef2c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Release History # Unreleased -- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `DeprecationWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened so fractional digits are not lost. +- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `DeprecationWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened so fractional digits are not lost (up to the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded). - Preserve the originating session when re-fetching or concurrently polling async statements on the kernel backend. # 4.8.0 (2026-10-24) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index cb636ce27..cba0a5d0b 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -623,7 +623,9 @@ def __init__( The `name` argument to this function would be `my_param`. :scale: The number of digits to the right of the decimal point. If the value itself - has more fractional digits, the value's scale is used so no digits are lost. + has more fractional digits, the value's scale is used so no digits are lost + (unless the value needs more than 38 digits in total; the excess fractional + digits are then rounded by the server). :precision: The minimum precision (total number of digits) of the number, between 1 and 38. If the value has more integer digits than `precision - scale` allows, or its @@ -652,7 +654,9 @@ def __init__( "digits after the decimal point, as in SQL DECIMAL(precision, scale). " "Swap your arguments; this will raise ValueError in a future " "release.".format(self.scale, self.precision, self.scale, self.precision), - DeprecationWarning, + # FutureWarning (not DeprecationWarning) so end users see it by + # default, not only when the call originates from __main__. + FutureWarning, stacklevel=2, ) self.scale, self.precision = self.precision, self.scale @@ -676,9 +680,13 @@ def _cast_expr(self): MAX_PRECISION = 38 def _effective_precision_and_scale(self, precision: int, scale: int): - """Widen a declared DECIMAL(precision, scale) so the bound value never loses + """Widen a declared DECIMAL(precision, scale) so the bound value does not lose fractional digits: the value's own scale is authoritative over an under-declared target scale. Integer-digit room from the declared type is kept. + + Exception: the total is capped at MAX_PRECISION (38) digits. If the value + itself needs more than 38 digits (e.g. a scale over 38), the cast is clamped + and the server rounds away the fractional digits that do not fit. """ try: sign, digits, exponent = self.value.as_tuple() @@ -691,11 +699,22 @@ def _effective_precision_and_scale(self, precision: int, scale: int): value_scale = max(-exponent, 0) value_int_digits = max(len(digits) + exponent, 0) - effective_scale = max(scale, value_scale) - int_digits = max(precision - scale, value_int_digits) - effective_precision = min(int_digits + effective_scale, self.MAX_PRECISION) - effective_scale = min(effective_scale, effective_precision) - return effective_precision, effective_scale + desired_scale = max(scale, value_scale) + desired_int_digits = max(precision - scale, value_int_digits) + + # Allocate the (at most MAX_PRECISION) digits by priority so that, when the + # cap is hit, the value's own digits win over extra declared room + # (similar to Spark's decimal precision adjustment): + # 1. the value's integer digits, 2. the value's fractional digits, + # 3. extra declared integer room, 4. extra declared fractional room. + int_digits = min(value_int_digits, self.MAX_PRECISION) + effective_scale = min(value_scale, self.MAX_PRECISION - int_digits) + remaining = self.MAX_PRECISION - int_digits - effective_scale + extra_int = min(desired_int_digits - int_digits, remaining) + int_digits += extra_int + remaining -= extra_int + effective_scale += min(desired_scale - effective_scale, remaining) + return int_digits + effective_scale, effective_scale def calculate_decimal_cast_string(self, input: decimal.Decimal) -> str: """Returns the smallest SQL cast argument that can contain the passed decimal diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index ab4d1bc08..06cb4386a 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -122,6 +122,11 @@ def test_calculate_decimal_cast_string(value, expected): (Decimal("0.123456789012345678901234567890123456"), 2, 10, "DECIMAL(38,36)"), # Widened scale is capped at the (capped) precision (Decimal("0.1234567890123456789012345678901234567890"), 2, 10, "DECIMAL(38,38)"), + # When the cap is hit, the value's integer digits win and extra declared + # scale shrinks (31 int + 1 frac digits fit losslessly in DECIMAL(38,7)) + (Decimal("1234567890123456789012345678901.5"), 8, 10, "DECIMAL(38,7)"), + # Value fills all 38 digits: declared extra scale is dropped entirely + (Decimal("1234567890123456789012345678901234567.8"), 8, 10, "DECIMAL(38,1)"), # NaN / Infinity leave the declared type untouched (Decimal("NaN"), 2, 10, "DECIMAL(10,2)"), (Decimal("Infinity"), 2, 10, "DECIMAL(10,2)"), @@ -137,7 +142,7 @@ def test_decimal_explicit_precision_scale_cast_string(value, scale, precision, e def test_decimal_scale_greater_than_precision_legacy_swapped(): """Callers using the old (swapped) convention keep working, with a deprecation warning.""" - with pytest.warns(DeprecationWarning, match="legacy"): + with pytest.warns(FutureWarning, match="legacy"): p = DecimalParameter(Decimal("123.45"), scale=10, precision=2) assert p.precision == 10 assert p.scale == 2 From 864acfa72b86b24366dac07646b2a60fabcd752c Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:09:26 +0000 Subject: [PATCH 05/14] ai: apply changes for #1006 (2 review threads) Addresses: - #4418398193 at CHANGELOG.md:4 - #4418398199 at src/databricks/sql/parameters/native.py:673 Signed-off-by: peco-engineer-bot[bot] --- CHANGELOG.md | 2 +- src/databricks/sql/parameters/native.py | 12 ++++++++++++ tests/unit/test_parameters.py | 17 ++++++++++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d55ef2c9..053803270 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Release History # Unreleased -- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `DeprecationWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened so fractional digits are not lost (up to the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded). +- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened so fractional digits are not lost (up to the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded). - Preserve the originating session when re-fetching or concurrently polling async statements on the kernel backend. # 4.8.0 (2026-10-24) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index cba0a5d0b..d6759de52 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -2,12 +2,15 @@ import datetime import decimal +import logging import warnings from enum import Enum, auto from typing import Optional, Sequence, Any, TYPE_CHECKING from databricks.sql.exc import NotSupportedError +logger = logging.getLogger(__name__) + if TYPE_CHECKING: # Type-annotation-only imports. ``from __future__ import annotations`` keeps # these out of the runtime import graph; the ``TSparkParameter*`` objects @@ -672,6 +675,15 @@ def valid_scale_and_precision(self): def _cast_expr(self): if self.scale is not None and self.precision is not None: precision, scale = self._effective_precision_and_scale(self.precision, self.scale) + if (precision, scale) != (self.precision, self.scale): + logger.debug( + "DecimalParameter declared DECIMAL(%d,%d) is bound as DECIMAL(%d,%d) " + "to fit the value's digits (precision capped at 38)", + self.precision, + self.scale, + precision, + scale, + ) # SQL DECIMAL type syntax is DECIMAL(precision, scale) return self.CAST_EXPR.format(precision, scale) else: diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index 06cb4386a..9d7c9a2db 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -140,8 +140,23 @@ def test_decimal_explicit_precision_scale_cast_string(value, scale, precision, e assert p._cast_expr() == expected +def test_decimal_widened_type_is_logged(caplog): + """A bound type that differs from the declared one is reported at debug level.""" + p = DecimalParameter(Decimal("123.4567"), scale=2, precision=10) + with caplog.at_level("DEBUG", logger="databricks.sql.parameters.native"): + assert p._cast_expr() == "DECIMAL(12,4)" + assert "DECIMAL(10,2) is bound as DECIMAL(12,4)" in caplog.text + + +def test_decimal_unchanged_type_is_not_logged(caplog): + p = DecimalParameter(Decimal("123.45"), scale=2, precision=10) + with caplog.at_level("DEBUG", logger="databricks.sql.parameters.native"): + assert p._cast_expr() == "DECIMAL(10,2)" + assert "is bound as" not in caplog.text + + def test_decimal_scale_greater_than_precision_legacy_swapped(): - """Callers using the old (swapped) convention keep working, with a deprecation warning.""" + """Callers using the old (swapped) convention keep working, with a FutureWarning.""" with pytest.warns(FutureWarning, match="legacy"): p = DecimalParameter(Decimal("123.45"), scale=10, precision=2) assert p.precision == 10 From 2823eed90385670bc4d73c45e1bc6b48cf1a6e49 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:13:43 +0000 Subject: [PATCH 06/14] ai: apply changes for #1006 (3 review threads) Addresses: - #4418440090 at src/databricks/sql/parameters/native.py:715 - #4418440099 at src/databricks/sql/parameters/native.py:702 - #4418440106 at src/databricks/sql/parameters/native.py:704 Signed-off-by: peco-engineer-bot[bot] --- CHANGELOG.md | 2 +- src/databricks/sql/parameters/native.py | 33 +++++++++++++++++++------ tests/unit/test_parameters.py | 27 +++++++++++++++++++- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 053803270..4417e9b0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Release History # Unreleased -- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened so fractional digits are not lost (up to the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded). +- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and the bound type is widened to fit the value: a declared `scale` smaller than the value's own scale is widened so fractional digits are not lost, and a declared `precision` whose integer room (`precision - scale`) is smaller than the value's integer digits is also widened, so such values are no longer rejected by the server as out of range (e.g. `DecimalParameter(Decimal("123456789012.5"), precision=10, scale=2)` now binds `DECIMAL(14,2)` instead of failing). If you relied on `precision` as a server-side range check, validate the value client-side. Widening is capped at the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded. - Preserve the originating session when re-fetching or concurrently polling async statements on the kernel backend. # 4.8.0 (2026-10-24) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index d6759de52..25e85fa41 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -664,6 +664,18 @@ def __init__( ) self.scale, self.precision = self.precision, self.scale + if self.scale is not None and self.precision is not None: + if self.scale < 0: + raise ValueError( + "DecimalParameter scale must be >= 0, got {}".format(self.scale) + ) + if not 1 <= self.precision <= self.MAX_PRECISION: + raise ValueError( + "DecimalParameter precision must be between 1 and {}, got {}".format( + self.MAX_PRECISION, self.precision + ) + ) + def valid_scale_and_precision(self): if (self.scale is None and self.precision is None) or ( isinstance(self.scale, int) and isinstance(self.precision, int) @@ -692,18 +704,25 @@ def _cast_expr(self): MAX_PRECISION = 38 def _effective_precision_and_scale(self, precision: int, scale: int): - """Widen a declared DECIMAL(precision, scale) so the bound value does not lose - fractional digits: the value's own scale is authoritative over an - under-declared target scale. Integer-digit room from the declared type is kept. + """Widen a declared DECIMAL(precision, scale) so the bound value fits: the + value's own scale is authoritative over an under-declared target scale, and + the integer-digit room is widened to the value's integer digits when the + declared ``precision - scale`` is too small (so the declared precision is not + enforced as a range check). Extra integer room from the declared type is kept. Exception: the total is capped at MAX_PRECISION (38) digits. If the value itself needs more than 38 digits (e.g. a scale over 38), the cast is clamped and the server rounds away the fractional digits that do not fit. """ - try: - sign, digits, exponent = self.value.as_tuple() - except AttributeError: - return precision, scale + value = self.value + if not isinstance(value, decimal.Decimal): + # The value is sent to the server as str(value), so inspect that same + # textual form; this makes int / float / str values widen like Decimals. + try: + value = decimal.Decimal(str(value)) + except (decimal.InvalidOperation, ValueError, TypeError): + return precision, scale + sign, digits, exponent = value.as_tuple() if not isinstance(exponent, int): # NaN / Infinity: leave the declared type as-is return precision, scale diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index 9d7c9a2db..9a3e99c72 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -130,8 +130,13 @@ def test_calculate_decimal_cast_string(value, expected): # NaN / Infinity leave the declared type untouched (Decimal("NaN"), 2, 10, "DECIMAL(10,2)"), (Decimal("Infinity"), 2, 10, "DECIMAL(10,2)"), - # Non-Decimal values (no as_tuple) fall back to the declared type + # Non-Decimal values are inspected via str(value), which is what is sent (1.5, 2, 10, "DECIMAL(10,2)"), + (12.3456, 2, 10, "DECIMAL(12,4)"), + (123456789012, 0, 10, "DECIMAL(12,0)"), + ("123.4567", 2, 10, "DECIMAL(12,4)"), + # Unparseable values fall back to the declared type + ("not-a-number", 2, 10, "DECIMAL(10,2)"), ), ) def test_decimal_explicit_precision_scale_cast_string(value, scale, precision, expected): @@ -164,6 +169,26 @@ def test_decimal_scale_greater_than_precision_legacy_swapped(): assert p._cast_expr() == "DECIMAL(10,2)" +@pytest.mark.parametrize( + "scale,precision,match", + ( + (2, 50, "precision must be between 1 and 38"), + (0, 0, "precision must be between 1 and 38"), + (-1, 10, "scale must be >= 0"), + ), +) +def test_decimal_invalid_declared_precision_or_scale_raises(scale, precision, match): + with pytest.raises(ValueError, match=match): + DecimalParameter(Decimal("1.5"), scale=scale, precision=precision) + + +def test_decimal_legacy_swapped_out_of_range_precision_raises(): + """The range check applies after the legacy swap (scale=50 becomes precision=50).""" + with pytest.warns(FutureWarning, match="legacy"): + with pytest.raises(ValueError, match="precision must be between 1 and 38"): + DecimalParameter(Decimal("1.5"), scale=50, precision=2) + + class Primitive(Enum): """These are the inferrable types. This Enum is used for parametrized tests.""" From 2518fc66969abe46ea26965a8657196f4966f5f4 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:19:03 +0000 Subject: [PATCH 07/14] ai: apply changes for #1006 (2 review threads) Addresses: - #4418478127 at src/databricks/sql/parameters/native.py:734 - #4418478139 at src/databricks/sql/parameters/native.py:678 Signed-off-by: peco-engineer-bot[bot] --- src/databricks/sql/parameters/native.py | 25 +++++++++++++++++++--- tests/unit/test_parameters.py | 28 +++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index 25e85fa41..735dce3ba 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -641,7 +641,8 @@ def __init__( if not self.valid_scale_and_precision(): raise ValueError( - "DecimalParameter requires both or none of scale and precision to be set" + "DecimalParameter requires both or none of scale and precision to be set, " + "as integers" ) if self.scale is not None and self.precision is not None and self.scale > self.precision: @@ -677,8 +678,13 @@ def __init__( ) def valid_scale_and_precision(self): + def _is_int(x): + # bool is a subclass of int; reject it so precision=True/scale=False + # cannot silently bind DECIMAL(1,0). + return isinstance(x, int) and not isinstance(x, bool) + if (self.scale is None and self.precision is None) or ( - isinstance(self.scale, int) and isinstance(self.precision, int) + _is_int(self.scale) and _is_int(self.precision) ): return True else: @@ -687,7 +693,20 @@ def valid_scale_and_precision(self): def _cast_expr(self): if self.scale is not None and self.precision is not None: precision, scale = self._effective_precision_and_scale(self.precision, self.scale) - if (precision, scale) != (self.precision, self.scale): + if precision - scale > self.precision - self.scale: + # The value has more integer digits than the declared type allows. + # Earlier releases let the server reject this as out of range, so + # surface it above DEBUG for callers relying on the declared type. + logger.warning( + "DecimalParameter value has more integer digits than declared " + "DECIMAL(%d,%d) allows; it is bound as DECIMAL(%d,%d) instead of " + "being rejected as out of range", + self.precision, + self.scale, + precision, + scale, + ) + elif (precision, scale) != (self.precision, self.scale): logger.debug( "DecimalParameter declared DECIMAL(%d,%d) is bound as DECIMAL(%d,%d) " "to fit the value's digits (precision capped at 38)", diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index 9a3e99c72..dc605a874 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -153,6 +153,24 @@ def test_decimal_widened_type_is_logged(caplog): assert "DECIMAL(10,2) is bound as DECIMAL(12,4)" in caplog.text +def test_decimal_widened_integer_room_is_warned(caplog): + """Widening the integer room (no longer an out-of-range error) is logged at WARNING.""" + p = DecimalParameter(Decimal("123456789012.5"), scale=2, precision=10) + with caplog.at_level("WARNING", logger="databricks.sql.parameters.native"): + assert p._cast_expr() == "DECIMAL(14,2)" + warnings = [r for r in caplog.records if r.levelname == "WARNING"] + assert len(warnings) == 1 + assert "DECIMAL(10,2) allows; it is bound as DECIMAL(14,2)" in warnings[0].getMessage() + + +def test_decimal_widened_scale_only_is_not_warned(caplog): + """Scale-only widening (the bug fix) stays at DEBUG, not WARNING.""" + p = DecimalParameter(Decimal("123.4567"), scale=2, precision=10) + with caplog.at_level("DEBUG", logger="databricks.sql.parameters.native"): + assert p._cast_expr() == "DECIMAL(12,4)" + assert not [r for r in caplog.records if r.levelname == "WARNING"] + + def test_decimal_unchanged_type_is_not_logged(caplog): p = DecimalParameter(Decimal("123.45"), scale=2, precision=10) with caplog.at_level("DEBUG", logger="databricks.sql.parameters.native"): @@ -182,6 +200,16 @@ def test_decimal_invalid_declared_precision_or_scale_raises(scale, precision, ma DecimalParameter(Decimal("1.5"), scale=scale, precision=precision) +@pytest.mark.parametrize( + "scale,precision", + ((False, True), (0, True), (False, 10)), +) +def test_decimal_bool_scale_or_precision_raises(scale, precision): + """bool is an int subclass but must not be accepted as a scale/precision.""" + with pytest.raises(ValueError, match="as integers"): + DecimalParameter(Decimal("1.5"), scale=scale, precision=precision) + + def test_decimal_legacy_swapped_out_of_range_precision_raises(): """The range check applies after the legacy swap (scale=50 becomes precision=50).""" with pytest.warns(FutureWarning, match="legacy"): From 6177ab4f3881f3e6890c2ff13c23abb19500b990 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:24:25 +0000 Subject: [PATCH 08/14] ai: apply changes for #1006 (3 review threads) Addresses: - #4418514374 at src/databricks/sql/parameters/native.py:648 - #4418514381 at src/databricks/sql/parameters/native.py:737 - #4418514389 at CHANGELOG.md:4 Signed-off-by: peco-engineer-bot[bot] --- CHANGELOG.md | 2 +- src/databricks/sql/parameters/native.py | 73 ++++++++++++------------- tests/unit/test_parameters.py | 37 +++++++------ 3 files changed, 57 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4417e9b0e..e08f29455 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Release History # Unreleased -- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and the bound type is widened to fit the value: a declared `scale` smaller than the value's own scale is widened so fractional digits are not lost, and a declared `precision` whose integer room (`precision - scale`) is smaller than the value's integer digits is also widened, so such values are no longer rejected by the server as out of range (e.g. `DecimalParameter(Decimal("123456789012.5"), precision=10, scale=2)` now binds `DECIMAL(14,2)` instead of failing). If you relied on `precision` as a server-side range check, validate the value client-side. Widening is capped at the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded. +- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened (growing the precision by the same amount) so fractional digits are not lost (e.g. `DecimalParameter(Decimal("123.4567"), precision=10, scale=2)` binds `DECIMAL(12,4)`). The declared integer room (`precision - scale`) is not widened, so a value with more integer digits is still rejected by the server as out of range. Widening is capped at the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded. The constructor now also raises `ValueError` for inputs it previously accepted: a `precision` outside 1..38 (e.g. `scale=0, precision=0`, which used to fall back to an inferred type), a negative `scale`, and `bool` values for `scale` or `precision`. - Preserve the originating session when re-fetching or concurrently polling async statements on the kernel backend. # 4.8.0 (2026-10-24) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index 735dce3ba..93de586ca 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -604,11 +604,13 @@ def __init__( If set, `scale` and `precision` must both be set. If neither is set, the value will be casted to the smallest possible DECIMAL type that can contain it. - When set, `precision` and `scale` declare a *minimum* DECIMAL(precision, scale) - type: if the value needs more fractional or integer digits than declared, the - bound type is widened to fit it (capped at precision 38) so no digits are lost. - The value is never rounded or truncated to a narrower declared type; round the - `Decimal` yourself (e.g. with `Decimal.quantize`) before binding if you need that. + When set, `precision` and `scale` declare a DECIMAL(precision, scale) type. If + the value has more fractional digits than `scale`, the bound scale (and so the + precision) is widened to fit them (capped at precision 38) so they are not lost; + the declared integer room (`precision - scale`) is kept, so a value with more + integer digits than that is still rejected by the server as out of range. + Round the `Decimal` yourself (e.g. with `Decimal.quantize`) before binding if + you need it narrowed to the declared scale. :value: The value to bind for this parameter. This will be casted to a DECIMAL. @@ -630,9 +632,9 @@ def __init__( (unless the value needs more than 38 digits in total; the excess fractional digits are then rounded by the server). :precision: - The minimum precision (total number of digits) of the number, between 1 and 38. - If the value has more integer digits than `precision - scale` allows, or its - scale is widened, the precision grows to fit it (up to 38). + The precision (total number of digits) of the number, between 1 and 38. + `precision - scale` integer digits are allowed; if the scale is widened to + fit the value, the precision grows by the same amount (up to 38). """ self.value: decimal.Decimal = value self.name = name @@ -645,7 +647,11 @@ def __init__( "as integers" ) - if self.scale is not None and self.precision is not None and self.scale > self.precision: + if ( + self.scale is not None + and self.precision is not None + and self.scale > self.precision + ): # Earlier releases documented these arguments the other way round # (scale = total digits, precision = fractional digits). A valid # DECIMAL can never have scale > precision, so this can only be a call @@ -657,7 +663,9 @@ def __init__( "`precision` is the total number of digits and `scale` is the number of " "digits after the decimal point, as in SQL DECIMAL(precision, scale). " "Swap your arguments; this will raise ValueError in a future " - "release.".format(self.scale, self.precision, self.scale, self.precision), + "release.".format( + self.scale, self.precision, self.scale, self.precision + ), # FutureWarning (not DeprecationWarning) so end users see it by # default, not only when the call originates from __main__. FutureWarning, @@ -692,24 +700,13 @@ def _is_int(x): def _cast_expr(self): if self.scale is not None and self.precision is not None: - precision, scale = self._effective_precision_and_scale(self.precision, self.scale) - if precision - scale > self.precision - self.scale: - # The value has more integer digits than the declared type allows. - # Earlier releases let the server reject this as out of range, so - # surface it above DEBUG for callers relying on the declared type. - logger.warning( - "DecimalParameter value has more integer digits than declared " - "DECIMAL(%d,%d) allows; it is bound as DECIMAL(%d,%d) instead of " - "being rejected as out of range", - self.precision, - self.scale, - precision, - scale, - ) - elif (precision, scale) != (self.precision, self.scale): + precision, scale = self._effective_precision_and_scale( + self.precision, self.scale + ) + if (precision, scale) != (self.precision, self.scale): logger.debug( "DecimalParameter declared DECIMAL(%d,%d) is bound as DECIMAL(%d,%d) " - "to fit the value's digits (precision capped at 38)", + "to fit the value's fractional digits (precision capped at 38)", self.precision, self.scale, precision, @@ -723,15 +720,15 @@ def _cast_expr(self): MAX_PRECISION = 38 def _effective_precision_and_scale(self, precision: int, scale: int): - """Widen a declared DECIMAL(precision, scale) so the bound value fits: the - value's own scale is authoritative over an under-declared target scale, and - the integer-digit room is widened to the value's integer digits when the - declared ``precision - scale`` is too small (so the declared precision is not - enforced as a range check). Extra integer room from the declared type is kept. + """Widen a declared DECIMAL(precision, scale) so the value's fractional digits + fit: the value's own scale is authoritative over an under-declared target + scale. The declared integer room (``precision - scale``) is never widened, so + a value with more integer digits is still rejected by the server as out of + range, as in earlier releases. Exception: the total is capped at MAX_PRECISION (38) digits. If the value - itself needs more than 38 digits (e.g. a scale over 38), the cast is clamped - and the server rounds away the fractional digits that do not fit. + needs more than that (e.g. a scale over 38), the cast is clamped and the + server rounds away the fractional digits that do not fit. """ value = self.value if not isinstance(value, decimal.Decimal): @@ -747,20 +744,22 @@ def _effective_precision_and_scale(self, precision: int, scale: int): return precision, scale value_scale = max(-exponent, 0) - value_int_digits = max(len(digits) + exponent, 0) + declared_int_digits = precision - scale + # Integer room is never widened past the declared one: an overflowing value + # is left for the server to reject as out of range. + value_int_digits = min(max(len(digits) + exponent, 0), declared_int_digits) desired_scale = max(scale, value_scale) - desired_int_digits = max(precision - scale, value_int_digits) # Allocate the (at most MAX_PRECISION) digits by priority so that, when the # cap is hit, the value's own digits win over extra declared room # (similar to Spark's decimal precision adjustment): # 1. the value's integer digits, 2. the value's fractional digits, # 3. extra declared integer room, 4. extra declared fractional room. - int_digits = min(value_int_digits, self.MAX_PRECISION) + int_digits = value_int_digits effective_scale = min(value_scale, self.MAX_PRECISION - int_digits) remaining = self.MAX_PRECISION - int_digits - effective_scale - extra_int = min(desired_int_digits - int_digits, remaining) + extra_int = min(declared_int_digits - int_digits, remaining) int_digits += extra_int remaining -= extra_int effective_scale += min(desired_scale - effective_scale, remaining) diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index dc605a874..0ed6b74f5 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -114,26 +114,31 @@ def test_calculate_decimal_cast_string(value, expected): (Decimal("123"), 0, 10, "DECIMAL(10,0)"), # Value scale exceeds declared scale: scale widens, declared int room (8) is kept (Decimal("123.4567"), 2, 10, "DECIMAL(12,4)"), - # Value has more integer digits than declared precision - scale allows - (Decimal("123456789012.5"), 2, 10, "DECIMAL(14,2)"), - # Positive exponent: 1E+3 needs 4 integer digits, no fractional digits - (Decimal("1E+3"), 0, 2, "DECIMAL(4,0)"), + # Value has more integer digits than declared precision - scale allows: + # integer room is not widened, so the server rejects it as out of range + (Decimal("123456789012.5"), 2, 10, "DECIMAL(10,2)"), + # Positive exponent: 1E+3 needs 4 integer digits; declared room (2) is kept + (Decimal("1E+3"), 0, 2, "DECIMAL(2,0)"), + (Decimal("1E+3"), 0, 5, "DECIMAL(5,0)"), # Widened precision is capped at 38 (Decimal("0.123456789012345678901234567890123456"), 2, 10, "DECIMAL(38,36)"), # Widened scale is capped at the (capped) precision (Decimal("0.1234567890123456789012345678901234567890"), 2, 10, "DECIMAL(38,38)"), - # When the cap is hit, the value's integer digits win and extra declared - # scale shrinks (31 int + 1 frac digits fit losslessly in DECIMAL(38,7)) - (Decimal("1234567890123456789012345678901.5"), 8, 10, "DECIMAL(38,7)"), - # Value fills all 38 digits: declared extra scale is dropped entirely - (Decimal("1234567890123456789012345678901234567.8"), 8, 10, "DECIMAL(38,1)"), + # When the cap is hit, the value's fractional digits win over extra declared + # integer room (1 int + 5 frac digits; 32 of the 36 declared int digits kept) + (Decimal("1.12345"), 2, 38, "DECIMAL(38,5)"), + # The value's integer digits win over its fractional digits at the cap + # (36 int digits fill the declared room; the server rounds the excess scale) + (Decimal("123456789012345678901234567890123456.12345"), 2, 38, "DECIMAL(38,2)"), + # Integer overflow of the declared room is never widened, even near the cap + (Decimal("1234567890123456789012345678901.5"), 8, 10, "DECIMAL(10,8)"), # NaN / Infinity leave the declared type untouched (Decimal("NaN"), 2, 10, "DECIMAL(10,2)"), (Decimal("Infinity"), 2, 10, "DECIMAL(10,2)"), # Non-Decimal values are inspected via str(value), which is what is sent (1.5, 2, 10, "DECIMAL(10,2)"), (12.3456, 2, 10, "DECIMAL(12,4)"), - (123456789012, 0, 10, "DECIMAL(12,0)"), + (123456789012, 0, 10, "DECIMAL(10,0)"), ("123.4567", 2, 10, "DECIMAL(12,4)"), # Unparseable values fall back to the declared type ("not-a-number", 2, 10, "DECIMAL(10,2)"), @@ -153,14 +158,12 @@ def test_decimal_widened_type_is_logged(caplog): assert "DECIMAL(10,2) is bound as DECIMAL(12,4)" in caplog.text -def test_decimal_widened_integer_room_is_warned(caplog): - """Widening the integer room (no longer an out-of-range error) is logged at WARNING.""" +def test_decimal_integer_room_is_not_widened(caplog): + """Too many integer digits keep the declared type so the server range check applies.""" p = DecimalParameter(Decimal("123456789012.5"), scale=2, precision=10) - with caplog.at_level("WARNING", logger="databricks.sql.parameters.native"): - assert p._cast_expr() == "DECIMAL(14,2)" - warnings = [r for r in caplog.records if r.levelname == "WARNING"] - assert len(warnings) == 1 - assert "DECIMAL(10,2) allows; it is bound as DECIMAL(14,2)" in warnings[0].getMessage() + with caplog.at_level("DEBUG", logger="databricks.sql.parameters.native"): + assert p._cast_expr() == "DECIMAL(10,2)" + assert "is bound as" not in caplog.text def test_decimal_widened_scale_only_is_not_warned(caplog): From 2b3a85a8d2d07fdc42e81fdce03e3a7a248ea483 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:28:10 +0000 Subject: [PATCH 09/14] ai: apply changes for #1006 (2 review threads) Addresses: - #4418552981 at src/databricks/sql/parameters/native.py:674 - #4418552988 at src/databricks/sql/parameters/native.py:677 Signed-off-by: peco-engineer-bot[bot] --- src/databricks/sql/parameters/native.py | 35 +++++++++++++++++++++++- tests/unit/test_parameters.py | 36 +++++++++++++++++++++---- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index 93de586ca..03d393a49 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -640,6 +640,7 @@ def __init__( self.name = name self.scale = scale self.precision = precision + self._legacy_swapped = False if not self.valid_scale_and_precision(): raise ValueError( @@ -647,6 +648,28 @@ def __init__( "as integers" ) + if self.scale is not None and self.precision is not None: + # Range-check the raw arguments *before* the legacy swap below, so a call + # that is simply invalid (rather than written for the legacy convention) + # gets an error naming the argument the caller actually passed, and no + # misleading migration warning. + if self.scale < 0: + raise ValueError( + "DecimalParameter scale must be >= 0, got {}".format(self.scale) + ) + if self.precision < 0 or self.precision > self.MAX_PRECISION: + raise ValueError( + "DecimalParameter precision must be between 1 and {}, got {}".format( + self.MAX_PRECISION, self.precision + ) + ) + if self.scale > self.MAX_PRECISION: + raise ValueError( + "DecimalParameter scale must be between 0 and {}, got {}".format( + self.MAX_PRECISION, self.scale + ) + ) + if ( self.scale is not None and self.precision is not None @@ -663,7 +686,10 @@ def __init__( "`precision` is the total number of digits and `scale` is the number of " "digits after the decimal point, as in SQL DECIMAL(precision, scale). " "Swap your arguments; this will raise ValueError in a future " - "release.".format( + "release. Legacy-convention calls keep the previous behaviour (the " + "server rounds the value to the declared scale); with correctly " + "ordered arguments the bound scale is widened to fit the value's " + "fractional digits instead.".format( self.scale, self.precision, self.scale, self.precision ), # FutureWarning (not DeprecationWarning) so end users see it by @@ -672,6 +698,9 @@ def __init__( stacklevel=2, ) self.scale, self.precision = self.precision, self.scale + # Preserve the old bound type exactly (no widening) for these callers, + # so the only change they see is the warning. + self._legacy_swapped = True if self.scale is not None and self.precision is not None: if self.scale < 0: @@ -700,6 +729,10 @@ def _is_int(x): def _cast_expr(self): if self.scale is not None and self.precision is not None: + if self._legacy_swapped: + # Legacy-convention call: bind the declared type unchanged, as + # earlier releases did, so existing rounding is preserved. + return self.CAST_EXPR.format(self.precision, self.scale) precision, scale = self._effective_precision_and_scale( self.precision, self.scale ) diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index 0ed6b74f5..4659378f4 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -1,4 +1,5 @@ import datetime +import warnings from decimal import Decimal from enum import Enum from typing import Type @@ -190,6 +191,22 @@ def test_decimal_scale_greater_than_precision_legacy_swapped(): assert p._cast_expr() == "DECIMAL(10,2)" +def test_decimal_legacy_swapped_is_not_widened(): + """Legacy-convention calls keep the old bound type (and so the old rounding): + DecimalParameter(Decimal("12.3456"), scale=4, precision=2) still binds DECIMAL(4,2) + rather than being widened to DECIMAL(6,4).""" + with pytest.warns(FutureWarning, match="previous behaviour"): + p = DecimalParameter(Decimal("12.3456"), scale=4, precision=2) + assert p._cast_expr() == "DECIMAL(4,2)" + assert p.as_tspark_param(named=False).type == "DECIMAL(4,2)" + + +def test_decimal_correct_order_with_more_fractional_digits_is_widened(): + """Counterpart of the legacy test: correctly ordered arguments do widen.""" + p = DecimalParameter(Decimal("12.3456"), scale=2, precision=4) + assert p._cast_expr() == "DECIMAL(6,4)" + + @pytest.mark.parametrize( "scale,precision,match", ( @@ -213,11 +230,20 @@ def test_decimal_bool_scale_or_precision_raises(scale, precision): DecimalParameter(Decimal("1.5"), scale=scale, precision=precision) -def test_decimal_legacy_swapped_out_of_range_precision_raises(): - """The range check applies after the legacy swap (scale=50 becomes precision=50).""" - with pytest.warns(FutureWarning, match="legacy"): - with pytest.raises(ValueError, match="precision must be between 1 and 38"): - DecimalParameter(Decimal("1.5"), scale=50, precision=2) +@pytest.mark.parametrize( + "scale,precision,match", + ( + (50, 2, "scale must be between 0 and 38, got 50"), + (5, -1, "precision must be between 1 and 38, got -1"), + ), +) +def test_decimal_out_of_range_raw_args_raise_before_legacy_swap(scale, precision, match): + """Range checks run on the raw arguments before the legacy swap, so the error + names the argument the caller actually passed and no FutureWarning is emitted.""" + with warnings.catch_warnings(): + warnings.simplefilter("error", FutureWarning) + with pytest.raises(ValueError, match=match): + DecimalParameter(Decimal("1.5"), scale=scale, precision=precision) class Primitive(Enum): From 25634d430f3a2dfdeaeb7a473e314e234424316b Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:32:27 +0000 Subject: [PATCH 10/14] ai: apply changes for #1006 (2 review threads) Addresses: - #4418588494 at src/databricks/sql/parameters/native.py:660 - #4418588499 at src/databricks/sql/parameters/native.py:705 Signed-off-by: peco-engineer-bot[bot] --- CHANGELOG.md | 2 +- src/databricks/sql/parameters/native.py | 20 +++++++------------- tests/unit/test_parameters.py | 3 +++ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e08f29455..757b646fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Release History # Unreleased -- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened (growing the precision by the same amount) so fractional digits are not lost (e.g. `DecimalParameter(Decimal("123.4567"), precision=10, scale=2)` binds `DECIMAL(12,4)`). The declared integer room (`precision - scale`) is not widened, so a value with more integer digits is still rejected by the server as out of range. Widening is capped at the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded. The constructor now also raises `ValueError` for inputs it previously accepted: a `precision` outside 1..38 (e.g. `scale=0, precision=0`, which used to fall back to an inferred type), a negative `scale`, and `bool` values for `scale` or `precision`. +- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened (growing the precision by the same amount) so fractional digits are not lost (e.g. `DecimalParameter(Decimal("123.4567"), precision=10, scale=2)` binds `DECIMAL(12,4)`). The declared integer room (`precision - scale`) is not widened, so a value with more integer digits is still rejected by the server as out of range. Widening is capped at the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded. The constructor now also raises `ValueError` for inputs it previously accepted: a `precision` outside 1..38 (including `precision=0`, e.g. `scale=0, precision=0` or `scale=5, precision=0`, which used to fall back to an inferred type), a negative `scale`, and `bool` values for `scale` or `precision`. - Preserve the originating session when re-fetching or concurrently polling async statements on the kernel backend. # 4.8.0 (2026-10-24) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index 03d393a49..dc00b97ed 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -657,7 +657,10 @@ def __init__( raise ValueError( "DecimalParameter scale must be >= 0, got {}".format(self.scale) ) - if self.precision < 0 or self.precision > self.MAX_PRECISION: + # precision=0 is rejected here too (not swapped): earlier releases + # treated it as unset and inferred the type, so swapping e.g. + # scale=5, precision=0 into DECIMAL(5,0) would silently round the value. + if not 1 <= self.precision <= self.MAX_PRECISION: raise ValueError( "DecimalParameter precision must be between 1 and {}, got {}".format( self.MAX_PRECISION, self.precision @@ -700,20 +703,11 @@ def __init__( self.scale, self.precision = self.precision, self.scale # Preserve the old bound type exactly (no widening) for these callers, # so the only change they see is the warning. + # No re-validation needed: the swapped scale is the raw precision + # (already 1..38) and the swapped precision is the raw scale (already + # > that and <= 38), so the result is always a valid DECIMAL. self._legacy_swapped = True - if self.scale is not None and self.precision is not None: - if self.scale < 0: - raise ValueError( - "DecimalParameter scale must be >= 0, got {}".format(self.scale) - ) - if not 1 <= self.precision <= self.MAX_PRECISION: - raise ValueError( - "DecimalParameter precision must be between 1 and {}, got {}".format( - self.MAX_PRECISION, self.precision - ) - ) - def valid_scale_and_precision(self): def _is_int(x): # bool is a subclass of int; reject it so precision=True/scale=False diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index 4659378f4..2bfa1f62c 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -235,6 +235,9 @@ def test_decimal_bool_scale_or_precision_raises(scale, precision): ( (50, 2, "scale must be between 0 and 38, got 50"), (5, -1, "precision must be between 1 and 38, got -1"), + # precision=0 used to mean "unset" (type inferred); it must not be swapped + # into DECIMAL(5,0), which would round the value. + (5, 0, "precision must be between 1 and 38, got 0"), ), ) def test_decimal_out_of_range_raw_args_raise_before_legacy_swap(scale, precision, match): From 1eff0199fb4d90970fa3b6765cbfdcce2f810dfe Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:36:08 +0000 Subject: [PATCH 11/14] ai: apply changes for #1006 (2 review threads) Addresses: - #4418612993 at src/databricks/sql/parameters/native.py:773 - #4418612995 at src/databricks/sql/parameters/native.py:663 Signed-off-by: peco-engineer-bot[bot] --- CHANGELOG.md | 2 +- src/databricks/sql/parameters/native.py | 31 ++++++++++++++++++++++--- tests/unit/test_parameters.py | 20 ++++++++++++---- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 757b646fa..abc2e1a74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Release History # Unreleased -- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened (growing the precision by the same amount) so fractional digits are not lost (e.g. `DecimalParameter(Decimal("123.4567"), precision=10, scale=2)` binds `DECIMAL(12,4)`). The declared integer room (`precision - scale`) is not widened, so a value with more integer digits is still rejected by the server as out of range. Widening is capped at the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded. The constructor now also raises `ValueError` for inputs it previously accepted: a `precision` outside 1..38 (including `precision=0`, e.g. `scale=0, precision=0` or `scale=5, precision=0`, which used to fall back to an inferred type), a negative `scale`, and `bool` values for `scale` or `precision`. +- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened (growing the precision by the same amount) so fractional digits are not lost (e.g. `DecimalParameter(Decimal("123.4567"), precision=10, scale=2)` binds `DECIMAL(12,4)`). The declared integer room (`precision - scale`) is not widened, so a value with more integer digits is still rejected by the server as out of range. Widening is capped at the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded. The constructor now also raises `ValueError` for inputs it previously accepted: a `precision` outside 0..38, a negative `scale`, and `bool` values for `scale` or `precision`. `precision=0` (e.g. `scale=5, precision=0`) still falls back to an inferred type as before, but now emits a `FutureWarning`; it will raise `ValueError` in a future release. - Preserve the originating session when re-fetching or concurrently polling async statements on the kernel backend. # 4.8.0 (2026-10-24) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index dc00b97ed..dbc533cee 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -648,6 +648,24 @@ def __init__( "as integers" ) + if self.precision == 0: + # Earlier releases treated precision=0 as falsy, i.e. "unset", and inferred + # the type from the value. Keep that behaviour for now (swapping e.g. + # scale=5, precision=0 into DECIMAL(5,0) would silently round the value), + # but warn so callers migrate before this becomes an error, like the + # other legacy-convention calls below. + warnings.warn( + "DecimalParameter received precision=0 (scale={}); ignoring scale and " + "precision and inferring the DECIMAL type from the value, as earlier " + "releases did. `precision` must be between 1 and {}; pass both as None " + "to infer the type. This will raise ValueError in a future " + "release.".format(self.scale, self.MAX_PRECISION), + FutureWarning, + stacklevel=2, + ) + self.scale = None + self.precision = None + if self.scale is not None and self.precision is not None: # Range-check the raw arguments *before* the legacy swap below, so a call # that is simply invalid (rather than written for the legacy convention) @@ -657,9 +675,6 @@ def __init__( raise ValueError( "DecimalParameter scale must be >= 0, got {}".format(self.scale) ) - # precision=0 is rejected here too (not swapped): earlier releases - # treated it as unset and inferred the type, so swapping e.g. - # scale=5, precision=0 into DECIMAL(5,0) would silently round the value. if not 1 <= self.precision <= self.MAX_PRECISION: raise ValueError( "DecimalParameter precision must be between 1 and {}, got {}".format( @@ -770,6 +785,16 @@ def _effective_precision_and_scale(self, precision: int, scale: int): # NaN / Infinity: leave the declared type as-is return precision, scale + # Trailing fractional zeros carry no information, so ignore them: otherwise + # Decimal("1.50") and Decimal("1.5") would bind different types for the same + # declared parameter. (Stripping keeps len(digits) + exponent unchanged.) + while exponent < 0 and len(digits) > 1 and digits[-1] == 0: + digits = digits[:-1] + exponent += 1 + if digits == (0,): + # Zero (e.g. Decimal("0.000")) has no significant fractional digits + exponent = max(exponent, 0) + value_scale = max(-exponent, 0) declared_int_digits = precision - scale # Integer room is never widened past the declared one: an overflowing value diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index 2bfa1f62c..4ff079369 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -115,6 +115,11 @@ def test_calculate_decimal_cast_string(value, expected): (Decimal("123"), 0, 10, "DECIMAL(10,0)"), # Value scale exceeds declared scale: scale widens, declared int room (8) is kept (Decimal("123.4567"), 2, 10, "DECIMAL(12,4)"), + # Trailing fractional zeros are not significant and do not widen the scale + (Decimal("123.4500"), 2, 10, "DECIMAL(10,2)"), + (Decimal("123.4560"), 2, 10, "DECIMAL(11,3)"), + (Decimal("0.000"), 2, 10, "DECIMAL(10,2)"), + (Decimal("100"), 0, 10, "DECIMAL(10,0)"), # Value has more integer digits than declared precision - scale allows: # integer room is not widened, so the server rejects it as out of range (Decimal("123456789012.5"), 2, 10, "DECIMAL(10,2)"), @@ -211,7 +216,6 @@ def test_decimal_correct_order_with_more_fractional_digits_is_widened(): "scale,precision,match", ( (2, 50, "precision must be between 1 and 38"), - (0, 0, "precision must be between 1 and 38"), (-1, 10, "scale must be >= 0"), ), ) @@ -235,9 +239,6 @@ def test_decimal_bool_scale_or_precision_raises(scale, precision): ( (50, 2, "scale must be between 0 and 38, got 50"), (5, -1, "precision must be between 1 and 38, got -1"), - # precision=0 used to mean "unset" (type inferred); it must not be swapped - # into DECIMAL(5,0), which would round the value. - (5, 0, "precision must be between 1 and 38, got 0"), ), ) def test_decimal_out_of_range_raw_args_raise_before_legacy_swap(scale, precision, match): @@ -249,6 +250,17 @@ def test_decimal_out_of_range_raw_args_raise_before_legacy_swap(scale, precision DecimalParameter(Decimal("1.5"), scale=scale, precision=precision) +@pytest.mark.parametrize("scale", (0, 5, 10)) +def test_decimal_precision_zero_falls_back_to_inferred_type_with_warning(scale): + """precision=0 used to mean "unset" (type inferred). Keep that, with a + FutureWarning, rather than raising or swapping into DECIMAL(scale,0), which + would round the value.""" + with pytest.warns(FutureWarning, match="precision=0"): + p = DecimalParameter(Decimal("1234.56"), scale=scale, precision=0) + assert p.scale is None and p.precision is None + assert p._cast_expr() == "DECIMAL(6,2)" + + class Primitive(Enum): """These are the inferrable types. This Enum is used for parametrized tests.""" From 9036107788dc33fe9ee759cfe589c4e8e38811e8 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:42:39 +0000 Subject: [PATCH 12/14] ai: apply changes for #1006 (1 review thread) Addresses: - #4418637929 at src/databricks/sql/parameters/native.py:740 Signed-off-by: peco-engineer-bot[bot] --- CHANGELOG.md | 2 +- src/databricks/sql/parameters/native.py | 65 +++++++++++++++++++------ tests/unit/test_parameters.py | 38 +++++++++++++-- 3 files changed, 86 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abc2e1a74..1c5da571c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Release History # Unreleased -- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened (growing the precision by the same amount) so fractional digits are not lost (e.g. `DecimalParameter(Decimal("123.4567"), precision=10, scale=2)` binds `DECIMAL(12,4)`). The declared integer room (`precision - scale`) is not widened, so a value with more integer digits is still rejected by the server as out of range. Widening is capped at the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded. The constructor now also raises `ValueError` for inputs it previously accepted: a `precision` outside 0..38, a negative `scale`, and `bool` values for `scale` or `precision`. `precision=0` (e.g. `scale=5, precision=0`) still falls back to an inferred type as before, but now emits a `FutureWarning`; it will raise `ValueError` in a future release. +- **Behaviour change:** `DecimalParameter` now binds `DECIMAL(precision, scale)`, matching SQL: `precision` is the total number of digits and `scale` the number of fractional digits (earlier docs described them the other way round). Calls written for the old convention (`scale > precision`, e.g. `scale=10, precision=2`) still bind `DECIMAL(10,2)` but emit a `FutureWarning`; swap your arguments, as this will raise `ValueError` in a future release. A declared `scale` equal to or less than `precision` is now interpreted with the SQL meaning, and a declared `scale` smaller than the value's own scale is widened (growing the precision by the same amount) so fractional digits are not lost (e.g. `DecimalParameter(Decimal("123.4567"), precision=10, scale=2)` binds `DECIMAL(12,4)`). The declared integer room (`precision - scale`) is not widened, so a value with more integer digits is still rejected by the server as out of range. Widening is capped at the 38-digit DECIMAL maximum; a value needing more digits than that still has its excess fractional digits rounded. The constructor now also raises `ValueError` for inputs it previously accepted: a `precision` outside 0..38, a negative `scale`, and `bool` values for `scale` or `precision`. `precision=0` (e.g. `scale=5, precision=0`) still falls back to an inferred type as before, but now emits a `FutureWarning`; it will raise `ValueError` in a future release. `scale=0` (previously also treated as unset) is now honoured as `DECIMAL(precision, 0)`, except that when the value has more integer digits than `precision` (which earlier releases accepted via the inferred type) it falls back to the inferred type with a `FutureWarning`; this will raise `ValueError` in a future release. - Preserve the originating session when re-fetching or concurrently polling async statements on the kernel backend. # 4.8.0 (2026-10-24) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index dbc533cee..2ac36ba78 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -608,7 +608,8 @@ def __init__( the value has more fractional digits than `scale`, the bound scale (and so the precision) is widened to fit them (capped at precision 38) so they are not lost; the declared integer room (`precision - scale`) is kept, so a value with more - integer digits than that is still rejected by the server as out of range. + integer digits than that is still rejected by the server as out of range + (except, for backward compatibility, with `scale=0`: see the CHANGELOG). Round the `Decimal` yourself (e.g. with `Decimal.quantize`) before binding if you need it narrowed to the declared scale. @@ -688,6 +689,31 @@ def __init__( ) ) + if self.scale == 0 and self.precision is not None: + # Earlier releases also treated scale=0 as falsy, i.e. "unset", and + # inferred the type from the value. Honouring DECIMAL(precision, 0) is + # only a regression when the value has more integer digits than + # `precision` (the server would now reject a call that used to work), so + # keep the inferred type for exactly that case, with a warning. + normalized = self._normalized_value_digits() + if normalized is not None: + digits, exponent = normalized + if max(len(digits) + exponent, 0) > self.precision: + warnings.warn( + "DecimalParameter received scale=0, precision={} for a value " + "with more than {} integer digits; ignoring scale and " + "precision and inferring the DECIMAL type from the value, as " + "earlier releases did. Declare a precision large enough for " + "the value, or pass both as None to infer the type. This " + "will raise ValueError in a future release.".format( + self.precision, self.precision + ), + FutureWarning, + stacklevel=2, + ) + self.scale = None + self.precision = None + if ( self.scale is not None and self.precision is not None @@ -761,17 +787,9 @@ def _cast_expr(self): MAX_PRECISION = 38 - def _effective_precision_and_scale(self, precision: int, scale: int): - """Widen a declared DECIMAL(precision, scale) so the value's fractional digits - fit: the value's own scale is authoritative over an under-declared target - scale. The declared integer room (``precision - scale``) is never widened, so - a value with more integer digits is still rejected by the server as out of - range, as in earlier releases. - - Exception: the total is capped at MAX_PRECISION (38) digits. If the value - needs more than that (e.g. a scale over 38), the cast is clamped and the - server rounds away the fractional digits that do not fit. - """ + def _normalized_value_digits(self): + """Return ``(digits, exponent)`` of the value with insignificant trailing + fractional zeros stripped, or None if the value is not a finite number.""" value = self.value if not isinstance(value, decimal.Decimal): # The value is sent to the server as str(value), so inspect that same @@ -779,11 +797,11 @@ def _effective_precision_and_scale(self, precision: int, scale: int): try: value = decimal.Decimal(str(value)) except (decimal.InvalidOperation, ValueError, TypeError): - return precision, scale + return None sign, digits, exponent = value.as_tuple() if not isinstance(exponent, int): - # NaN / Infinity: leave the declared type as-is - return precision, scale + # NaN / Infinity + return None # Trailing fractional zeros carry no information, so ignore them: otherwise # Decimal("1.50") and Decimal("1.5") would bind different types for the same @@ -794,6 +812,23 @@ def _effective_precision_and_scale(self, precision: int, scale: int): if digits == (0,): # Zero (e.g. Decimal("0.000")) has no significant fractional digits exponent = max(exponent, 0) + return digits, exponent + + def _effective_precision_and_scale(self, precision: int, scale: int): + """Widen a declared DECIMAL(precision, scale) so the value's fractional digits + fit: the value's own scale is authoritative over an under-declared target + scale. The declared integer room (``precision - scale``) is never widened, so + a value with more integer digits is still rejected by the server as out of + range, as in earlier releases. + + Exception: the total is capped at MAX_PRECISION (38) digits. If the value + needs more than that (e.g. a scale over 38), the cast is clamped and the + server rounds away the fractional digits that do not fit. + """ + normalized = self._normalized_value_digits() + if normalized is None: + return precision, scale + digits, exponent = normalized value_scale = max(-exponent, 0) declared_int_digits = precision - scale diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index 4ff079369..432a30322 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -123,9 +123,11 @@ def test_calculate_decimal_cast_string(value, expected): # Value has more integer digits than declared precision - scale allows: # integer room is not widened, so the server rejects it as out of range (Decimal("123456789012.5"), 2, 10, "DECIMAL(10,2)"), - # Positive exponent: 1E+3 needs 4 integer digits; declared room (2) is kept - (Decimal("1E+3"), 0, 2, "DECIMAL(2,0)"), + # Positive exponent: 1E+3 needs 4 integer digits, which DECIMAL(5,0) fits (Decimal("1E+3"), 0, 5, "DECIMAL(5,0)"), + (Decimal("1E+3"), 0, 4, "DECIMAL(4,0)"), + # scale=0 with a fractional value that fits the integer room: honoured, widened + (Decimal("123.45"), 0, 3, "DECIMAL(5,2)"), # Widened precision is capped at 38 (Decimal("0.123456789012345678901234567890123456"), 2, 10, "DECIMAL(38,36)"), # Widened scale is capped at the (capped) precision @@ -144,7 +146,7 @@ def test_calculate_decimal_cast_string(value, expected): # Non-Decimal values are inspected via str(value), which is what is sent (1.5, 2, 10, "DECIMAL(10,2)"), (12.3456, 2, 10, "DECIMAL(12,4)"), - (123456789012, 0, 10, "DECIMAL(10,0)"), + (1234567890, 0, 10, "DECIMAL(10,0)"), ("123.4567", 2, 10, "DECIMAL(12,4)"), # Unparseable values fall back to the declared type ("not-a-number", 2, 10, "DECIMAL(10,2)"), @@ -261,6 +263,36 @@ def test_decimal_precision_zero_falls_back_to_inferred_type_with_warning(scale): assert p._cast_expr() == "DECIMAL(6,2)" +@pytest.mark.parametrize( + "value,precision,expected", + ( + (Decimal("123.45"), 2, "DECIMAL(5,2)"), + (Decimal("1E+3"), 2, "DECIMAL(4,0)"), + (123456789012, 10, "DECIMAL(12,0)"), + ), +) +def test_decimal_scale_zero_integer_overflow_falls_back_to_inferred_type( + value, precision, expected +): + """scale=0 used to mean "unset" (type inferred). When honouring DECIMAL(precision,0) + would overflow the value's integer digits (a call that used to succeed), keep the + inferred type, with a FutureWarning.""" + with pytest.warns(FutureWarning, match="scale=0"): + p = DecimalParameter(value, scale=0, precision=precision) + assert p.scale is None and p.precision is None + assert p._cast_expr() == expected + + +@pytest.mark.parametrize( + "value,precision", ((Decimal("123.45"), 3), (Decimal("0.5"), 1), (Decimal("NaN"), 1)) +) +def test_decimal_scale_zero_that_fits_is_honoured_without_warning(value, precision): + with warnings.catch_warnings(): + warnings.simplefilter("error", FutureWarning) + p = DecimalParameter(value, scale=0, precision=precision) + assert (p.scale, p.precision) == (0, precision) + + class Primitive(Enum): """These are the inferrable types. This Enum is used for parametrized tests.""" From 1bda507961b6c337f8d14823747e162bfee5a55a Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:46:43 +0000 Subject: [PATCH 13/14] ai: apply changes for #1006 (1 review thread) Addresses: - #4418689780 at src/databricks/sql/parameters/native.py:652 Signed-off-by: peco-engineer-bot[bot] --- src/databricks/sql/parameters/native.py | 26 ++++++++++++++----------- tests/unit/test_parameters.py | 16 +++++++++++++++ 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index 2ac36ba78..929c4d33e 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -649,6 +649,20 @@ def __init__( "as integers" ) + if self.scale is not None: + # Range-check scale before the precision=0 fallback below, so a clearly + # invalid scale is reported rather than hidden behind a migration warning. + if self.scale < 0: + raise ValueError( + "DecimalParameter scale must be >= 0, got {}".format(self.scale) + ) + if self.scale > self.MAX_PRECISION: + raise ValueError( + "DecimalParameter scale must be between 0 and {}, got {}".format( + self.MAX_PRECISION, self.scale + ) + ) + if self.precision == 0: # Earlier releases treated precision=0 as falsy, i.e. "unset", and inferred # the type from the value. Keep that behaviour for now (swapping e.g. @@ -671,23 +685,13 @@ def __init__( # Range-check the raw arguments *before* the legacy swap below, so a call # that is simply invalid (rather than written for the legacy convention) # gets an error naming the argument the caller actually passed, and no - # misleading migration warning. - if self.scale < 0: - raise ValueError( - "DecimalParameter scale must be >= 0, got {}".format(self.scale) - ) + # misleading migration warning. (scale was range-checked above.) if not 1 <= self.precision <= self.MAX_PRECISION: raise ValueError( "DecimalParameter precision must be between 1 and {}, got {}".format( self.MAX_PRECISION, self.precision ) ) - if self.scale > self.MAX_PRECISION: - raise ValueError( - "DecimalParameter scale must be between 0 and {}, got {}".format( - self.MAX_PRECISION, self.scale - ) - ) if self.scale == 0 and self.precision is not None: # Earlier releases also treated scale=0 as falsy, i.e. "unset", and diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index 432a30322..d29f24d32 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -263,6 +263,22 @@ def test_decimal_precision_zero_falls_back_to_inferred_type_with_warning(scale): assert p._cast_expr() == "DECIMAL(6,2)" +@pytest.mark.parametrize( + "scale,match", + ( + (-1, "scale must be >= 0, got -1"), + (500, "scale must be between 0 and 38, got 500"), + ), +) +def test_decimal_precision_zero_with_invalid_scale_raises(scale, match): + """An out-of-range scale is rejected even with precision=0, not hidden behind + the precision=0 migration warning.""" + with warnings.catch_warnings(): + warnings.simplefilter("error", FutureWarning) + with pytest.raises(ValueError, match=match): + DecimalParameter(Decimal("1.5"), scale=scale, precision=0) + + @pytest.mark.parametrize( "value,precision,expected", ( From 97b8acaae85c942ffd6c84f8436afb924b0e4656 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 29 Oct 2026 22:50:35 +0000 Subject: [PATCH 14/14] ai: apply changes for #1006 (1 review thread) Addresses: - #4418708511 at src/databricks/sql/parameters/native.py:724 Signed-off-by: peco-engineer-bot[bot] --- src/databricks/sql/parameters/native.py | 3 +++ tests/unit/test_parameters.py | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/databricks/sql/parameters/native.py b/src/databricks/sql/parameters/native.py index 929c4d33e..44e49b5e7 100644 --- a/src/databricks/sql/parameters/native.py +++ b/src/databricks/sql/parameters/native.py @@ -728,6 +728,9 @@ def __init__( # DECIMAL can never have scale > precision, so this can only be a call # written for the old convention: keep it working for now (swap the # arguments) but warn so callers migrate before this becomes an error. + # scale == precision is deliberately *not* handled here: it reads the + # same under both conventions, so it gets the SQL meaning and is widened + # like any correctly ordered call (documented in the CHANGELOG). warnings.warn( "DecimalParameter received scale ({}) > precision ({}); interpreting the " "arguments using the legacy (swapped) convention as DECIMAL({},{}). " diff --git a/tests/unit/test_parameters.py b/tests/unit/test_parameters.py index d29f24d32..a83665e72 100644 --- a/tests/unit/test_parameters.py +++ b/tests/unit/test_parameters.py @@ -214,6 +214,17 @@ def test_decimal_correct_order_with_more_fractional_digits_is_widened(): assert p._cast_expr() == "DECIMAL(6,4)" +def test_decimal_scale_equal_to_precision_is_widened_without_warning(): + """scale == precision means the same in both conventions, so it is not treated as + a legacy call: it gets the SQL meaning and is widened like any correctly ordered + call (earlier releases bound DECIMAL(2,2) and rounded to 0.12). See CHANGELOG.""" + with warnings.catch_warnings(): + warnings.simplefilter("error") + p = DecimalParameter(Decimal("0.1234"), scale=2, precision=2) + assert not p._legacy_swapped + assert p._cast_expr() == "DECIMAL(4,4)" + + @pytest.mark.parametrize( "scale,precision,match", (