Skip to content

Fix SQLColumnCheckOperator crash on non-numeric column bounds - #70895

Merged
potiuk merged 1 commit into
apache:mainfrom
shahar1:fix/sql-column-check-non-numeric-bounds
Aug 1, 2026
Merged

Fix SQLColumnCheckOperator crash on non-numeric column bounds#70895
potiuk merged 1 commit into
apache:mainfrom
shahar1:fix/sql-column-check-non-numeric-bounds

Conversation

@shahar1

@shahar1 shahar1 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

SQLColumnCheckOperator checks declared on a date or text column — {"min": {"geq_to": "2020-01-01"}}, {"max": {"leq_to": date(2020, 12, 31)}} — now fail the task with TypeError: unsupported operand type(s) for -: 'str' and 'int'.

#69736 rewrote _get_match so the tolerance margin widens the bound outward for negative expected values, which is correct, but it routed every bound through arithmetic. With no tolerance configured the margin is 0, and bound - 0 still runs — on a bound that cannot be subtracted from. These checks compared cleanly before.

The same rewrite turned equal_to into a degenerate range, so an equal_to check with accept_none=False and a NULL result stopped reporting a failed check and started raising on None <= bound instead.

This keeps the abs()-margin semantics when a tolerance is set and compares the bound directly when it is not.

Verified as a regression: the added tests all pass on 69736's parent commit and all fail on main.

related: #69736


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 5)

Generated-by: Claude Code (Opus 5) following the guidelines

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.

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both regressions reproduce exactly as described, and the fix is scoped to the path that broke rather than reverting the earlier work.

The non-numeric bound case is the sharper of the two: returning 0 from the margin helper still meant bound - 0 executed, so a perfectly ordinary check like {"min": {"geq_to": "2020-01-01"}} on a date or text column started failing with a TypeError naming types the user never wrote:

'2020-01-01'                -> unsupported operand type(s) for -: 'str' and 'int'
datetime.date(2020, 12, 31) -> unsupported operand type(s) for -: 'datetime.date' and 'int'

Returning the bound untouched when no tolerance is configured is the right shape — no arithmetic at all, rather than arithmetic that happens to be a no-op for numbers.

The equal_to case matters just as much even though it is quieter. As a degenerate range with margin = 0, a NULL result hit expected <= None <= expected and raised, so accept_none=False stopped reporting a failed check and started crashing the task instead. record == expected gives False, which is the failed check the option is meant to produce.

Good that the abs() widening from #69736 is preserved for the tolerance path — this narrows the fix to the no-tolerance branch instead of undoing the negative-threshold handling.


Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

@potiuk
potiuk merged commit 3b2445a into apache:main Aug 1, 2026
106 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants