-
Notifications
You must be signed in to change notification settings - Fork 3k
TIME_MILLIS support to GenericParquetReader (issue #502) #1031
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
Conversation
|
Could you add a test for this? |
done; pls check out |
| required(116, "dec_38_10", Types.DecimalType.of(38, 10)), // maximum precision | ||
| required(117, "time", Types.TimeType.get()) | ||
| required(117, "time", Types.TimeType.get()), | ||
| required(118, "time_ms", Types.TimeType.get()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Iceberg won't actually write data in milliseconds, so we would need to write a Parquet file using the annotation and confirm that a time column is correct when reading.
Can you remove this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rdblue do we have any boilerplate code I can use to do that?
…in milliseconds, so we would need to write a Parquet file using the annotation and confirm that a time column is correct when reading.', so this test won't work.
|
We don't have anything currently, but you can use |
|
@pranaydharmale, I went ahead and merged this since it looks correct. We can follow up with that test. |
added support for TIME_MILLIS to GenericParquetReader.