Skip to content

Commit 2518fc6

Browse files
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] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 2823eed commit 2518fc6

2 files changed

Lines changed: 50 additions & 3 deletions

File tree

‎src/databricks/sql/parameters/native.py‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,8 @@ def __init__(
641641

642642
if not self.valid_scale_and_precision():
643643
raise ValueError(
644-
"DecimalParameter requires both or none of scale and precision to be set"
644+
"DecimalParameter requires both or none of scale and precision to be set, "
645+
"as integers"
645646
)
646647

647648
if self.scale is not None and self.precision is not None and self.scale > self.precision:
@@ -677,8 +678,13 @@ def __init__(
677678
)
678679

679680
def valid_scale_and_precision(self):
681+
def _is_int(x):
682+
# bool is a subclass of int; reject it so precision=True/scale=False
683+
# cannot silently bind DECIMAL(1,0).
684+
return isinstance(x, int) and not isinstance(x, bool)
685+
680686
if (self.scale is None and self.precision is None) or (
681-
isinstance(self.scale, int) and isinstance(self.precision, int)
687+
_is_int(self.scale) and _is_int(self.precision)
682688
):
683689
return True
684690
else:
@@ -687,7 +693,20 @@ def valid_scale_and_precision(self):
687693
def _cast_expr(self):
688694
if self.scale is not None and self.precision is not None:
689695
precision, scale = self._effective_precision_and_scale(self.precision, self.scale)
690-
if (precision, scale) != (self.precision, self.scale):
696+
if precision - scale > self.precision - self.scale:
697+
# The value has more integer digits than the declared type allows.
698+
# Earlier releases let the server reject this as out of range, so
699+
# surface it above DEBUG for callers relying on the declared type.
700+
logger.warning(
701+
"DecimalParameter value has more integer digits than declared "
702+
"DECIMAL(%d,%d) allows; it is bound as DECIMAL(%d,%d) instead of "
703+
"being rejected as out of range",
704+
self.precision,
705+
self.scale,
706+
precision,
707+
scale,
708+
)
709+
elif (precision, scale) != (self.precision, self.scale):
691710
logger.debug(
692711
"DecimalParameter declared DECIMAL(%d,%d) is bound as DECIMAL(%d,%d) "
693712
"to fit the value's digits (precision capped at 38)",

‎tests/unit/test_parameters.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,24 @@ def test_decimal_widened_type_is_logged(caplog):
153153
assert "DECIMAL(10,2) is bound as DECIMAL(12,4)" in caplog.text
154154

155155

156+
def test_decimal_widened_integer_room_is_warned(caplog):
157+
"""Widening the integer room (no longer an out-of-range error) is logged at WARNING."""
158+
p = DecimalParameter(Decimal("123456789012.5"), scale=2, precision=10)
159+
with caplog.at_level("WARNING", logger="databricks.sql.parameters.native"):
160+
assert p._cast_expr() == "DECIMAL(14,2)"
161+
warnings = [r for r in caplog.records if r.levelname == "WARNING"]
162+
assert len(warnings) == 1
163+
assert "DECIMAL(10,2) allows; it is bound as DECIMAL(14,2)" in warnings[0].getMessage()
164+
165+
166+
def test_decimal_widened_scale_only_is_not_warned(caplog):
167+
"""Scale-only widening (the bug fix) stays at DEBUG, not WARNING."""
168+
p = DecimalParameter(Decimal("123.4567"), scale=2, precision=10)
169+
with caplog.at_level("DEBUG", logger="databricks.sql.parameters.native"):
170+
assert p._cast_expr() == "DECIMAL(12,4)"
171+
assert not [r for r in caplog.records if r.levelname == "WARNING"]
172+
173+
156174
def test_decimal_unchanged_type_is_not_logged(caplog):
157175
p = DecimalParameter(Decimal("123.45"), scale=2, precision=10)
158176
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
182200
DecimalParameter(Decimal("1.5"), scale=scale, precision=precision)
183201

184202

203+
@pytest.mark.parametrize(
204+
"scale,precision",
205+
((False, True), (0, True), (False, 10)),
206+
)
207+
def test_decimal_bool_scale_or_precision_raises(scale, precision):
208+
"""bool is an int subclass but must not be accepted as a scale/precision."""
209+
with pytest.raises(ValueError, match="as integers"):
210+
DecimalParameter(Decimal("1.5"), scale=scale, precision=precision)
211+
212+
185213
def test_decimal_legacy_swapped_out_of_range_precision_raises():
186214
"""The range check applies after the legacy swap (scale=50 becomes precision=50)."""
187215
with pytest.warns(FutureWarning, match="legacy"):

0 commit comments

Comments
 (0)