Skip to content

Add checked time arithmetic regression coverage for scalar UDF type recovery #25925

Description

@Toby1009

Is your feature request related to a problem or challenge?

Follow-up to the review of #25668.

The time ± interval SQL tests added there still pass when the central type-recovery fallback is removed. SQL-planned arithmetic uses fail_on_overflow = false, and the unbounded result range already makes the overflow rule return Unordered, whether the function's range type is Time64 or Null.

With checked arithmetic, however, the recovered type is needed to recognize that time-of-day arithmetic wraps across midnight. A local ablation experiment confirmed that removing the fallback makes checked date_trunc('hour', t) + INTERVAL '2 hours' incorrectly report Ordered.

The merged fallback handles this correctly; this issue tracks stronger regression coverage and more precise documentation.

Describe the solution you'd like

  • Add a unit test constructing date_trunc('hour', t) + INTERVAL '2 hours' as a BinaryExpr with with_fail_on_overflow(true) over an ordered Time64(Nanosecond) child, and assert Unordered.
  • Verify that the test fails when the central fallback is removed. Keep the existing SQL result and plan tests.
  • Clarify that type recovery applies to any unbounded Null interval, including one returned by an override, while other intervals are preserved. Retain the original interval if the resolved return type cannot be represented by Interval::make_unbounded.
  • Describe the unchanged path as direct calls to PhysicalExpr::evaluate_bounds, rather than implying that scalar UDFs participate in the normal constraint-solver path. check_support currently excludes them.
  • Consider separating the existing bounds-preservation and error-propagation test cases for readability, as suggested in review.

Describe alternatives you've considered

Relying only on the existing SQL tests does not catch this regression: removing the fallback leaves their results and sorts unchanged. Checked arithmetic exposes the type-dependent time wrapping rule.

Additional context

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions