Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix year 0 is out of range in test_from_json_struct_timestamp #9972

Merged
merged 4 commits into from
Dec 7, 2023
Merged

Fix year 0 is out of range in test_from_json_struct_timestamp #9972

merged 4 commits into from
Dec 7, 2023

Conversation

thirtiseven
Copy link
Collaborator

Fixes #9936

Add regex to generate 0002 to 9999:

([0-9]{3}[2-9]|([1-9][0-9]{2}|0[1-9][0-9]|00[1-9])[0-1])

Breakdown:

[0-9]{4} = [0-9]{3}[2-9] + [0-9]{3}[0-1]

[0-9]{3}[0-1] = [1-9][0-9]{2}[0-1] + 0[0-9]{2}[0-1]

0[0-9]{2}[0-1] = 0[1-9][0-9][0-1] + 00[0-9][0-1]

00[0-9][0-1] = 00[1-9][0-1] + 000[0-1] (we don't want this)

so the left part is:

 [0-9]{3}[2-9] + [1-9][0-9]{2}[0-1] + 0[1-9][0-9][0-1] + 00[1-9][0-1]
= ([0-9]{3}[2-9]|([1-9][0-9]{2}|0[1-9][0-9]|00[1-9])[0-1])

Verified it's correct by checking all values with sre_yield.

This PR also moves similar regexps to data_gen.py for reuse.

@thirtiseven
Fix year 0 is out of range for test_from_json_struct_timestamp
5e1229a 
Signed-off-by: Haoyang Li <haoyangl@nvidia.com>
@thirtiseven
clean up
eb19108 
Signed-off-by: Haoyang Li <haoyangl@nvidia.com>
@thirtiseven thirtiseven added the test Only impacts tests label Dec 6, 2023
@thirtiseven thirtiseven self-assigned this Dec 6, 2023
@thirtiseven
Copy link
Collaborator Author

About the 'year 0 is out of range' error, I checked all StringGen usages in our IT, here are some suspicious cases:

  • test_cast_string_date_fallback
  • all cases use str_date_and_format_gen
  • test_explain_set_config

I will try to test and fix them, but maybe not here because they can't be solved by the regexp we already have.

res-life
res-life reviewed Dec 6, 2023
View reviewed changes
integration_tests/src/main/python/data_gen.py
# date related regexps for generating date strings within python's range limits

date_start_1_2_1 = '(0{0,3}1-(0?[2-9]|[1-3][0-9]))|(([0-9]{0,3}[2-9]|[1-9][0-9]{0,2}[01])-[0-3]?[0-9])-[0-5]?[0-9]'
Copy link
Collaborator

@res-life res-life Dec 6, 2023 •
edited

Choose a reason for hiding this comment

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

Add comment like: regexp to generate date from 0001-02-01, format is yyyy-MM-dd

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

res-life
res-life reviewed Dec 6, 2023
View reviewed changes
integration_tests/src/main/python/data_gen.py Outdated
date_start_1_2_1 = '(0{0,3}1-(0?[2-9]|[1-3][0-9]))|(([0-9]{0,3}[2-9]|[1-9][0-9]{0,2}[01])-[0-3]?[0-9])-[0-5]?[0-9]'

yyyy_start_0003 = '([0-9]{3}[2-9]|([1-9][0-9]{2}|0[1-9][0-9]|00[1-9])[0-1])'
Copy link
Collaborator

Choose a reason for hiding this comment

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

start from 0002 or 0003?
Add comment like: regexp to generate year from 0002, forat is yyyy

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nice catch, done.

@thirtiseven
address comments
7e75ca2 
Signed-off-by: Haoyang Li <haoyangl@nvidia.com>
@res-life
Copy link
Collaborator

res-life commented Dec 6, 2023

LGTM

res-life
res-life previously approved these changes Dec 6, 2023
View reviewed changes
@thirtiseven
Copy link
Collaborator Author

build

winningsix
winningsix reviewed Dec 6, 2023
View reviewed changes
integration_tests/src/main/python/data_gen.py Outdated
date_start_1_2_1 = '(0{0,3}1-(0?[2-9]|[1-3][0-9]))|(([0-9]{0,3}[2-9]|[1-9][0-9]{0,2}[01])-[0-3]?[0-9])-[0-5]?[0-9]'

# regexp to generate year from 0002, forat is yyyy
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: format

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW, why starting from 0002?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think 0001 will be convert to 0001-01-01 as timestamp in this case, which is out of range.

sperlingxx
sperlingxx reviewed Dec 6, 2023
View reviewed changes
integration_tests/src/main/python/data_gen.py Outdated

# regexp to generate year from 0002, forat is yyyy
yyyy_start_0002 = '([0-9]{3}[2-9]|([1-9][0-9]{2}|0[1-9][0-9]|00[1-9])[0-1])'
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: empty tailing line is missed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@thirtiseven
Copy link
Collaborator Author

premerge failed because of the rounding necessary issue.

@thirtiseven
fix nit and typo
bca3fbc 
Signed-off-by: Haoyang Li <haoyangl@nvidia.com>
@thirtiseven
Copy link
Collaborator Author

build

@thirtiseven
Copy link
Collaborator Author

rounding necessary issue again...

@thirtiseven
Copy link
Collaborator Author

build

@winningsix winningsix merged commit 6c5fbac into NVIDIA:branch-24.02 Dec 7, 2023
38 checks passed
@thirtiseven thirtiseven deleted the timestamp121 branch December 19, 2023 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sperlingxx sperlingxx sperlingxx left review comments

@winningsix winningsix winningsix approved these changes

@res-life res-life res-life left review comments

Assignees

@thirtiseven thirtiseven

Labels
test Only impacts tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Nightly CI of non-UTC time zone reports 'year 0 is out of range' error
4 participants
@thirtiseven @res-life @winningsix @sperlingxx