-
Notifications
You must be signed in to change notification settings - Fork 1k
Support different TimeUnits and timezones when reading Timestamps from INT96 #7285
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
Changes from all commits
72d2a37
e5a6363
ee320cc
0a82215
43f8ad0
be475dc
10844a1
8383c36
9cfa58b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -701,7 +701,7 @@ impl Field { | |
| /// `Timestamp` value. | ||
| #[inline] | ||
| pub fn convert_int96(_descr: &ColumnDescPtr, value: Int96) -> Self { | ||
| Field::TimestampMillis(value.to_i64()) | ||
| Field::TimestampMillis(value.to_millis()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be to_nanos() to preserve the old behavior But then again it doesn't make sense to erturn a nanosecond timestamp for a value with millisecond precision 🤔
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The old behavior is actually to convert it to millis. Current behavior for |
||
| } | ||
|
|
||
| /// Converts Parquet FLOAT type with logical type into `f32` value. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.