-
Notifications
You must be signed in to change notification settings - Fork 231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update test case related to LEACY datetime format to unblock nightly CI #11545
Conversation
Signed-off-by: Chong Gao <[email protected]>
For branch 24.12 branch, refer to #11544 |
build |
def test_yyyyMMdd_format_for_legacy_mode(): | ||
gen = StringGen("[0-9]{3}[1-9](0[1-9]|1[0-2])(0[1-9]|[1-2][0-9])") | ||
gen = StringGen('(19[0-9]{2}|[2-9][0-9]{3})([0-9]{4})') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: (19[0-9]{2}|[2-9][0-9]{3})([0-9]{4})
seems not only modifying the year format, but also changes the mmdd
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, do not need to change mmdd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you didnt replying my question... You changed the mmdd
in this change, was this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the previous reply.
You changed the mmdd in this change, was this intentional?
Yes, changed MM, and it's intentional.
The yyyy
years after 1900 are OK.
For MM
, invalid values like 99 month will result in NULL.
For dd
, also any values [0-9]{2}
is OK.
This PR is changing branch 24.10, and PR is changing 24.12.
On branch 24.12, it tests yyyymmdd
beside of yyyyMMdd
, and test case name renamed from test_yyyyMMdd_format_for_legacy_mode
to test_formats_for_legacy_mode
mm: Minute.
MM: month.
So we will use the changes on 24.12.
And we expect the auto-merge from 24.10 to 24.12 will fall.
Will conflict, refer to 24.12 changes: #11544 |
closes #11543
Changes:
Update test case: only test years after 1900
Description
This issue is caused by year 1582.
Spark uses hybrid Julian+Gregorian calendar for LEGACY mode.
This issue is related to LEGACY mode, Spark itself has different behaviors between LEGACY mode and non-LEGACY mode, and Rapids kernel has not 100% matched non-LEGACY yet.
Rapids keeps consistent with Spark when mode is CORRECTED mode.
We already documented that LEGACY mode has several limitations
So only update test case.
Signed-off-by: Chong Gao [email protected]