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.

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

[RFC 80] toDate can overflow #85

Open
shaobo-he-aws opened this issue Oct 25, 2024 · 6 comments
Open

[RFC 80] toDate can overflow #85

shaobo-he-aws opened this issue Oct 25, 2024 · 6 comments

Comments

@shaobo-he-aws
Copy link
Contributor

In RFC 80 (i.e., the datetime extension), toDate is proposed as a method of datetime. If its meaning is that it should return the date where the instant is, then it could overflow. A counterexample is datetime("1970-01-01").offset(duration("-9223372036854775808ms")).toDate(), which should return the date 106751991168 (9223372036854775808/86400000 + 1) days before the Unix epoch, and this date cannot be represented by a long.

@apg
Copy link
Contributor

apg commented Oct 29, 2024

I think the answer to this is that we bound duration to be something less than 292 million years. Creating a datetime, via the constructor, is already bounded to years 0-9999 by virtue of it being a 4 digit year requirement. We can say that 10,000 years is the duration limit, and fail on overflow.

@emina
Copy link
Contributor

emina commented Oct 31, 2024

To retain analyzability, it is important for every long (64-bit integer) to represent a legal datetime and/or duration.

If we don't do that, we are effectively introducing a subtype of integers ("those that represent legal dates / durations"), and this form of subtyping is not something we can express in the encoding.

So, let's allow all valid 64-bit values as durations, and throw overflow errors when calculations overflow.

@andrewmwells-amazon
Copy link
Contributor

I think the answer to this is that we bound duration to be something less than 292 million years. Creating a datetime, via the constructor, is already bounded to years 0-9999 by virtue of it being a 4 digit year requirement. We can say that 10,000 years is the duration limit, and fail on overflow.

Unless I'm mistaken, you can write something like DT.offset("10000y").offset("10000y")... so restricting the constructor doesn't actually restrict the range of valid DTs. I agree with Emina that we should allow all valid 64-bit values for both durations and DateTimes.

@apg
Copy link
Contributor

apg commented Nov 1, 2024

Stacking .offset("10000y") would cause the datetime to fall out of the 10,000 year range, and be considered "overflow."

I guess expanding the definition of what consitutes overflow to another 292 million years is fine, though. :)

@shaobo-he-aws
Copy link
Contributor Author

shaobo-he-aws commented Nov 1, 2024

Stacking .offset("10000y") would cause the datetime to fall out of the 10,000 year range, and be considered "overflow."

IIUC, what @emina suggests is that there should not be any restrictions on the datetime other than it must be representable by a long. So we cannot establish a 10000 year range.

@apg
Copy link
Contributor

apg commented Nov 1, 2024

@shaobo-he-aws: correct. I was responding to the objection that the 10,000 range is easy to circumvent.

By officially supporting this range, we'll be able to market Cedar to new applications -- paleontology (at least up to the Mesozoic era, or so).

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

No branches or pull requests

4 participants