You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "marvinlanhenke (via GitHub)" <gi...@apache.org> on 2024/03/31 20:56:18 UTC
[I] Bug: `fn day_timestamp_micro` produces wrong results [iceberg-rust]
marvinlanhenke opened a new issue, #311:
URL: https://github.com/apache/iceberg-rust/issues/311
### Description:
`fn day_timestamp_micro` ([source](https://github.com/apache/iceberg-rust/blob/main/crates/iceberg/src/transform/temporal.rs#L167)) seems to produce the wrong result which is off-by-one due to rounding errors.
Also the implementation does not align with the [Java ](https://github.com/apache/iceberg/blob/d350c9b8c995a2953aa8b80a0a1fc7cadc4dd16a/api/src/main/java/org/apache/iceberg/util/DateTimeUtil.java#L174-L184C7)implementation, which handles the conversion slightly different.
An example of the wrong output can be seen in the [test](https://github.com/apache/iceberg-rust/blob/main/crates/iceberg/src/transform/temporal.rs#L587):
`-115200000000` evaluates to `1969-12-30T16:00:00Z` which should be days -2 (since epoch) and not -1.
### Proposal:
Align Rust with Java implementation.
I already have a prototype, due to working on #264, so I will implement the fix in a different PR - so it can be verified more easily.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Bug: `fn day_timestamp_micro` produces wrong results [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #311:
URL: https://github.com/apache/iceberg-rust/issues/311#issuecomment-2029056694
Hi, @marvinlanhenke Thanks for the debug, yes we should align with java/python implementation carefully.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Bug: `fn day_timestamp_micro` produces wrong results [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 closed issue #311: Bug: `fn day_timestamp_micro` produces wrong results
URL: https://github.com/apache/iceberg-rust/issues/311
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Bug: `fn day_timestamp_micro` produces wrong results [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #311:
URL: https://github.com/apache/iceberg-rust/issues/311#issuecomment-2029287945
> @liurenjie1024 ...while looking at it - I think we need to handle `Year, Month, and Hours` as well? The Java implementation does the same thing for all granularities.
Yes, exactly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Bug: `fn day_timestamp_micro` produces wrong results [iceberg-rust]
Posted by "marvinlanhenke (via GitHub)" <gi...@apache.org>.
marvinlanhenke commented on issue #311:
URL: https://github.com/apache/iceberg-rust/issues/311#issuecomment-2029173981
@liurenjie1024
I'll try to get to submit a PR this evening. Still need to handle unwrap properly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Bug: `fn day_timestamp_micro` produces wrong results [iceberg-rust]
Posted by "marvinlanhenke (via GitHub)" <gi...@apache.org>.
marvinlanhenke commented on issue #311:
URL: https://github.com/apache/iceberg-rust/issues/311#issuecomment-2029198854
@liurenjie1024
...while looking at it - I think we need to handle `Year, Month, and Hours` as well? The Java implementation does the same thing for all granularities.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Bug: `fn day_timestamp_micro` produces wrong results [iceberg-rust]
Posted by "liurenjie1024 (via GitHub)" <gi...@apache.org>.
liurenjie1024 commented on issue #311:
URL: https://github.com/apache/iceberg-rust/issues/311#issuecomment-2029057550
Do you mind to submit a pr to fix it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [I] Bug: `fn day_timestamp_micro` produces wrong results [iceberg-rust]
Posted by "marvinlanhenke (via GitHub)" <gi...@apache.org>.
marvinlanhenke commented on issue #311:
URL: https://github.com/apache/iceberg-rust/issues/311#issuecomment-2028903379
cc @liurenjie1024
Prototype for the fix can be seen in #309, however I still have to handle unwrap and change the return type to `Result<i32>`. This should be enough, however, to verify if the logic is correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org