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

Interval Literal doesn't work for timeunit less than millisecond #3204

Closed
Tracked by #3148
waitingkuo opened this issue Aug 19, 2022 · 1 comment · Fixed by #4186
Closed
Tracked by #3148

Interval Literal doesn't work for timeunit less than millisecond #3204

waitingkuo opened this issue Aug 19, 2022 · 1 comment · Fixed by #4186
Labels
bug Something isn't working

Comments

@waitingkuo
Copy link
Contributor

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

It works for millisecond

select interval '1 month 0.001 second';
+-------------------------------------------------------+
| IntervalMonthDayNano("79228162514264337593544950336") |
+-------------------------------------------------------+
| 0 years 1 mons 0 days 0 hours 0 mins 0.1000000 secs   |
+-------------------------------------------------------+
1 row in set. Query took 0.001 seconds.

but doesn't work while the unit for second less than millisecond

select interval '1 month 0.0001 second';
+------------------------------------------------+
| IntervalYearMonth("1")                         |
+------------------------------------------------+
| 0 years 1 mons 0 days 0 hours 0 mins 0.00 secs |
+------------------------------------------------+
1 row in set. Query took 0.002 seconds.

Expected behavior
A clear and concise description of what you expected to happen.

Additional context
Add any other context about the problem here.

@waitingkuo waitingkuo added the bug Something isn't working label Aug 19, 2022
@Jefffrey
Copy link
Contributor

Jefffrey commented Nov 12, 2022

Seems to be because of this parsing:

https://github.com/apache/arrow-datafusion/blob/129654cd8466e7634bf186782c3d271541f0671e/datafusion/common/src/parsers.rs#L130-L161

Finest granularity is milliseconds, accumulated as an integer, hence fractions of that are silently ignored.

Could change this accumulator to nanoseconds, as i64, however this seems it would conflict with ScalarValue::IntervalDayTime which stores milliseconds as i32, compared to ScalarValue::IntervalMonthDayNano which does store nanoseconds.

Might need to change ScalarValue::IntervalDayTime to support nanoseconds perhaps? If want to be consistent between precision of ScalarValue::IntervalMonthDayNano and ScalarValue::IntervalDayTime

Edit: though since IntervalDayTime is an Arrow type, maybe just have that if fractions of milliseconds is found, then the result should always be a IntervalMonthDayNano to support keeping this precision, and thus not forcing IntervalDayTime to silently drop those fractional parts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants