Skip to content

fix: handle out of range errors in DATE_BIN instead of panicking#20221

Open
mishop-15 wants to merge 3 commits intoapache:mainfrom
mishop-15:fix-date-bin-panic
Open

fix: handle out of range errors in DATE_BIN instead of panicking#20221
mishop-15 wants to merge 3 commits intoapache:mainfrom
mishop-15:fix-date-bin-panic

Conversation

@mishop-15
Copy link
Contributor

Which issue does this PR close?

Closes #20219

Rationale for this change

The DATE_BIN function was panicking when datetime operations went out of range instead of returning proper errors. The two specific cases were:

  1. Month subtraction going out of range causing DateTime - Months panic
  2. timestamp_nanos_opt() returning None and then unwrapping

What changes are included in this PR?

  • Changed date_bin_months_interval and to_utc_date_time to return Result instead of panicking
  • Replaced origin_date - Months and origin_date + Months with checked_sub_months and checked_add_months
  • Replaced .unwrap() calls with proper match statements and error handling
  • Updated all callers throughout the file to handle Result types

Are these changes tested?

Tested manually with the exact queries from the issue that were panicking:

select DATE_BIN('1637426858', TO_TIMESTAMP_MILLIS(1040292460), TIMESTAMP '1984-01-07 00:00:00');
select DATE_BIN('1637426858', TO_TIMESTAMP_MILLIS(-1040292460), TIMESTAMP '1984-01-07 00:00:00');

Both queries now return NULL instead of panicking. All existing unit tests pass.

Are there any user-facing changes?

Yes - queries with DATE_BIN that would previously panic now return NULL when datetime operations go out of range.

@github-actions github-actions bot added the functions Changes to functions implementation label Feb 8, 2026
Copy link
Contributor

@neilconway neilconway left a comment

Choose a reason for hiding this comment

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

Thanks for looking at this! Can we add some test cases?

@mishop-15
Copy link
Contributor Author

Thanks for looking at this! Can we add some test cases?

Added test cases for both out-of-range scenarios in the latest commit.

stride: i64,
stride_fn: fn(i64, i64, i64) -> i64,
) -> impl Fn(i64) -> i64 {
stride_fn: fn(i64, i64, i64) -> Result<i64>,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Use BinFunction here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use BinFunction alias. Thanks for the review.

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

Labels

functions Changes to functions implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

date_bin() panics on large inputs

2 participants