Skip to content

fix: date validation reject invalid dates instead of silent normalization#315

Open
dalingmeng wants to merge 6 commits into
alibaba:mainfrom
dalingmeng:fix/date-validation
Open

fix: date validation reject invalid dates instead of silent normalization#315
dalingmeng wants to merge 6 commits into
alibaba:mainfrom
dalingmeng:fix/date-validation

Conversation

@dalingmeng
Copy link
Copy Markdown
Contributor

Purpose

no linked issue:

std::get_time only validates field ranges (e.g., tm_mday in [1, 31]) but does not check actual date validity (e.g., Feb 29 in non-leap year, Apr 31). After parsing, timegm/mktime silently normalizes these invalid dates instead of returning an error:

  • StringToDate("2019-02-29") → returns 17956 (2019-03-01) instead of error
  • StringToTimestampMillis("2023-02-30 00:00:00") → returns 1677686400000 (2023-03-02) instead of error

This change adds post-normalization validation: after calling timegm/mktime, we compare tm_mday and tm_mon with the original values. If they differ, the input date was invalid and an error is returned.

This aligns with Java Paimon behavior, where LocalDate.parse("2019-02-29") throws DateTimeParseException.

Tests

  • StringUtilsTest.TestStringToDateWithInvalidLeapYear — verifies Feb 29 in non-leap year, Feb 30, Apr 31 are rejected; valid leap year date (2020-02-29) succeeds
  • StringUtilsTest.TestStringToTimestampMillisWithInvalidDate — verifies Feb 30, Feb 29 in non-leap year, Apr 31 are rejected; valid leap year timestamp (2024-02-29) succeeds

API and Format

No API or format changes.

Documentation

No new features introduced.

Generative AI tooling

@dalingmeng dalingmeng force-pushed the fix/date-validation branch from 8d7d311 to e63e92e Compare May 28, 2026 09:26
Comment thread src/paimon/common/utils/string_utils.cpp
@dalingmeng dalingmeng force-pushed the fix/date-validation branch from e63e92e to 8b753ed Compare May 28, 2026 09:39
Copy link
Copy Markdown
Collaborator

@lxy-9602 lxy-9602 left a comment

Choose a reason for hiding this comment

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

+1

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants