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

feat: date/timestamp bound check #5054

Merged
merged 6 commits into from
Apr 27, 2022
Merged

feat: date/timestamp bound check #5054

merged 6 commits into from
Apr 27, 2022

Conversation

Veeupup
Copy link
Contributor

@Veeupup Veeupup commented Apr 26, 2022 •
edited
Loading

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

date/timestamp bound check

  • precison only support 0~6
  • date ranges from 1000-01-01 to 9999-12-31
  • timestamp ranges from 1000-01-01 00:00:00 to 9999-12-31 23:59:59

Changelog

  • Feature

Related Issues

Fixes #4874

Veeupup added 2 commits April 25, 2022 18:45
@Veeupup
upgrade and patch chrono
84e0cd9 
Signed-off-by: Veeupup <code@tanweime.com>
@Veeupup
add date/timestamp bound
70fc3a1 
Signed-off-by: Veeupup <code@tanweime.com>
@vercel
Copy link

vercel bot commented Apr 26, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
databend ⬜️ Ignored (Inspect) Apr 27, 2022 at 11:01AM (UTC)

@mergify
Copy link
Contributor

mergify bot commented Apr 26, 2022

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

Veeupup added 3 commits April 26, 2022 21:45
@Veeupup
timestamp bound check when deserializing
6e92aa8 
Signed-off-by: Veeupup <code@tanweime.com>
@Veeupup
date bound check when deserializing
ee7ce20 
Signed-off-by: Veeupup <code@tanweime.com>
@Veeupup
add bound check for date func
c268cd4
@Veeupup Veeupup marked this pull request as ready for review April 27, 2022 06:51
@Veeupup Veeupup requested a review from BohuTANG as a code owner April 27, 2022 06:51
@Veeupup Veeupup requested review from sundy-li and b41sh April 27, 2022 06:51
@Veeupup Veeupup force-pushed the datetime_bound_check branch 2 times, most recently from d2cc806 to 5ad99f8 Compare April 27, 2022 06:59
@Veeupup Veeupup marked this pull request as draft April 27, 2022 07:03
@Veeupup Veeupup marked this pull request as ready for review April 27, 2022 07:05
@Veeupup Veeupup mentioned this pull request Apr 27, 2022
Closed
@BohuTANG
Copy link
Member

Hmm, we also need bound checks for datetime functions, like select subtractYears(totimestamp(18875), 2); will panic the server.

@Veeupup
Copy link
Contributor Author

Veeupup commented Apr 27, 2022

@BohuTANG yes, it has been done and some stateless-test has been added.

@Veeupup
bound check: add stateless-test
a4c2671 
Signed-off-by: Veeupup <code@tanweime.com>
@Veeupup Veeupup force-pushed the datetime_bound_check branch from 5ad99f8 to a4c2671 Compare April 27, 2022 11:01
@BohuTANG BohuTANG merged commit f9046eb into databendlabs:main Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@b41sh b41sh b41sh left review comments

@sundy-li sundy-li sundy-li left review comments

@BohuTANG BohuTANG BohuTANG approved these changes

Assignees
No one assigned
Labels
need-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Datetime type attempt to multiply with overflow from chrono
5 participants
@Veeupup @BohuTANG @b41sh @sundy-li @databend-bot