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

DateTime.jd ignores fractional seconds #8034

Open
jeremyevans opened this issue Dec 7, 2023 · 3 comments
Open

DateTime.jd ignores fractional seconds #8034

jeremyevans opened this issue Dec 7, 2023 · 3 comments

Comments

@jeremyevans
Copy link
Contributor

Environment Information

Happens with both:

jruby 9.4.2.0 (3.1.0) 2023-03-08 90d2913fda OpenJDK 64-Bit Server VM 20.0.1+9-29 on 20.0.1+9-29 +jit [x86_64-mswin32]
jruby 9.4.5.0 (3.1.4) 2023-11-02 1abae2700f OpenJDK 64-Bit Server VM 17.0.9+9-1 on 17.0.9+9-1 +jit [x86_64-OpenBSD]

Expected Behavior

Respects fractional seconds, as in CRuby:

$ ruby -r date -e "p DateTime.jd(2454984, 15, 57, 3/2r).sec_fraction"
(1/2)

Actual Behavior

Ignores fractional seconds:

$ jruby -r date -e "p DateTime.jd(2454984, 15, 57, 3/2r).sec_fraction"
(0/1)

Found by Sequel's tests after a refactor.

@headius
Copy link
Member

headius commented Dec 7, 2023

I wonder if this is an out of date stdlib.

@headius
Copy link
Member

headius commented Dec 7, 2023

Welp, there's yer problem right there:

public static IRubyObject _valid_time_p(ThreadContext context, IRubyObject self,
IRubyObject h, IRubyObject m, IRubyObject s) {
long hour = normIntValue(h, 24);
long min = normIntValue(m, 60);
long sec = normIntValue(s, 60);
if (valid_time_p(hour, min, sec)) {
return timeToDayFraction(context, (int) hour, (int) min, (int) sec);
}
return context.nil;
}

We normalize seconds into a long, so there's no where for a rational to go. This logic is just out of date since CRuby started accepting Rational here.

@headius
Copy link
Member

headius commented Dec 7, 2023

The CRuby code starts here and involves many macros and local variable twiddling... so I did not start a port attempt. It's clear this logic has grown and we'll need to expand our handling of the time elements accordingly.

https://github.com/ruby/ruby/blob/7ffee5681f85de3fe74c25a90e05e31616113123/ext/date/date_core.c#L7659-L7717

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

2 participants