Skip to content

Commit 3b2445a

Browse files
authored
Fix SQLColumnCheckOperator crash on non-numeric column bounds (#70895)
The tolerance rewrite made every bound go through arithmetic, including when no tolerance is configured. A min/max check declared on a date or text column has a bound that cannot be subtracted from, so checks that compared cleanly before now fail the task with a TypeError. An equal_to check also stopped reporting a NULL result as a failed check and started raising instead, because a degenerate range still orders the record against the bound.
1 parent 0191b11 commit 3b2445a

2 files changed

Lines changed: 47 additions & 12 deletions

File tree

  • providers/common/sql
    • src/airflow/providers/common/sql/operators
    • tests/unit/common/sql/operators

providers/common/sql/src/airflow/providers/common/sql/operators/sql.py

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -746,25 +746,30 @@ def _get_match(self, check_values, record, tolerance=None) -> bool:
746746
match_boolean = True
747747

748748
# abs() so the tolerance margin widens the bound outward for negative expected values too.
749-
def _margin(expected):
750-
return abs(expected) * tolerance if tolerance is not None else 0
749+
# Without a tolerance the bound is compared as-is: a min/max check may be declared on a
750+
# date or text column, where arithmetic on the bound raises TypeError.
751+
def _lower(expected):
752+
return expected - abs(expected) * tolerance if tolerance is not None else expected
753+
754+
def _upper(expected):
755+
return expected + abs(expected) * tolerance if tolerance is not None else expected
751756

752757
if "geq_to" in check_values:
753-
match_boolean = record >= check_values["geq_to"] - _margin(check_values["geq_to"])
758+
match_boolean = record >= _lower(check_values["geq_to"])
754759
elif "greater_than" in check_values:
755-
match_boolean = record > check_values["greater_than"] - _margin(check_values["greater_than"])
760+
match_boolean = record > _lower(check_values["greater_than"])
756761
if "leq_to" in check_values:
757-
match_boolean = (
758-
record <= check_values["leq_to"] + _margin(check_values["leq_to"]) and match_boolean
759-
)
762+
match_boolean = record <= _upper(check_values["leq_to"]) and match_boolean
760763
elif "less_than" in check_values:
761-
match_boolean = (
762-
record < check_values["less_than"] + _margin(check_values["less_than"]) and match_boolean
763-
)
764+
match_boolean = record < _upper(check_values["less_than"]) and match_boolean
764765
if "equal_to" in check_values:
765766
expected = check_values["equal_to"]
766-
margin = _margin(expected)
767-
match_boolean = (expected - margin <= record <= expected + margin) and match_boolean
767+
if tolerance is None:
768+
# Equality, not a degenerate range: a NULL result must fail the check rather
769+
# than raise on an ordering comparison against None.
770+
match_boolean = record == expected and match_boolean
771+
else:
772+
match_boolean = (_lower(expected) <= record <= _upper(expected)) and match_boolean
768773
return match_boolean
769774

770775
def _column_mapping_validation(self, check, check_values):

providers/common/sql/tests/unit/common/sql/operators/test_sql.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1721,6 +1721,36 @@ def test_get_match_tolerance_handles_negative_thresholds(self, check_values, rec
17211721
op = self._make_operator({"col": {"min": {"geq_to": 1}}})
17221722
assert op._get_match(check_values, record, tolerance) == expected
17231723

1724+
@pytest.mark.parametrize(
1725+
("check_values", "record", "expected"),
1726+
[
1727+
({"geq_to": "2020-01-01"}, "2021-06-30", True),
1728+
({"geq_to": "2020-01-01"}, "2019-12-31", False),
1729+
({"greater_than": "2020-01-01"}, "2020-01-01", False),
1730+
({"leq_to": "2020-01-01"}, "2019-12-31", True),
1731+
({"less_than": "2020-01-01"}, "2020-01-01", False),
1732+
({"equal_to": "abc"}, "abc", True),
1733+
({"equal_to": "abc"}, "abcd", False),
1734+
(
1735+
{"geq_to": datetime.date(2020, 1, 1), "leq_to": datetime.date(2020, 12, 31)},
1736+
datetime.date(2020, 6, 30),
1737+
True,
1738+
),
1739+
(
1740+
{"geq_to": datetime.date(2020, 1, 1), "leq_to": datetime.date(2020, 12, 31)},
1741+
datetime.date(2021, 1, 1),
1742+
False,
1743+
),
1744+
],
1745+
)
1746+
def test_get_match_compares_non_numeric_bounds_without_tolerance(self, check_values, record, expected):
1747+
op = self._make_operator({"col": {"min": {"geq_to": 1}}})
1748+
assert op._get_match(check_values, record) == expected
1749+
1750+
def test_get_match_equal_to_fails_cleanly_on_none_record(self):
1751+
op = self._make_operator({"col": {"null_check": {"equal_to": 0}}}, accept_none=False)
1752+
assert op._get_match({"equal_to": 0}, None) is False
1753+
17241754
def test_multiple_checks_correct_names_and_order(self):
17251755
op = self._make_operator(
17261756
{

0 commit comments

Comments
 (0)