-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Set Calendar type used for temporal types in ORC footer #26507
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
Set Calendar type used for temporal types in ORC footer #26507
Conversation
f733ee2
to
cbb51f9
Compare
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.
Pull Request Overview
This PR adds support for setting the calendar type in ORC file footers for temporal data types. When writing ORC files, the system now detects if temporal types (DATE, TIMESTAMP, TIMESTAMP_INSTANT) are present and sets the appropriate calendar metadata in the footer.
Key changes:
- Added
CalendarKind
enum with three calendar types: UNKNOWN_CALENDAR, JULIAN_GREGORIAN, and PROLEPTIC_GREGORIAN - Enhanced Footer class to include optional calendar metadata
- Implemented logic to automatically set PROLEPTIC_GREGORIAN calendar when temporal types are detected during ORC writing
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
CalendarKind.java | New enum defining the three supported calendar types |
Footer.java | Added optional calendar field to store calendar metadata |
OrcMetadataReader.java | Added conversion from protobuf CalendarKind to Trino CalendarKind |
OrcMetadataWriter.java | Added conversion from Trino CalendarKind to protobuf and footer writing logic |
OrcType.java | Added constant set defining temporal ORC types |
OrcWriter.java | Added logic to detect temporal types and set PROLEPTIC_GREGORIAN calendar |
TestOrcWriter.java | Added comprehensive tests for calendar footer writing functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
lib/trino-orc/src/main/java/io/trino/orc/metadata/OrcMetadataReader.java
Show resolved
Hide resolved
lib/trino-orc/src/main/java/io/trino/orc/metadata/OrcMetadataWriter.java
Show resolved
Hide resolved
cbb51f9
to
f181afe
Compare
f181afe
to
be4ac5f
Compare
be4ac5f
to
aa57717
Compare
lib/trino-orc/src/main/java/io/trino/orc/metadata/OrcMetadataReader.java
Outdated
Show resolved
Hide resolved
aa57717
to
bfcee07
Compare
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.
Everything looks good. The only point left is the decision around reading, which I expect should not significantly impact the PR’s structure
I'm planning to continue work on the issue this week. |
5d27751
to
daabbcf
Compare
...ct-tests/src/main/java/io/trino/tests/product/hive/TestHiveOnOrcLegacyDateCompatibility.java
Outdated
Show resolved
Hide resolved
...ct-tests/src/main/java/io/trino/tests/product/hive/TestHiveOnOrcLegacyDateCompatibility.java
Outdated
Show resolved
Hide resolved
...ct-tests/src/main/java/io/trino/tests/product/hive/TestHiveOnOrcLegacyDateCompatibility.java
Show resolved
Hide resolved
daabbcf
to
593e7f0
Compare
593e7f0
to
5c98822
Compare
Description
The aim of this change is to add to the ORC file's footer the calendar type that was used to represent the dates or timestamps. Trino uses the Proleptic Gregorian calendar. The motivation behind is compatibility of that ORC files written by Trino to be properly readable by other tools like Apache Hive that can read ORC files.
Follow-up issue: #26865
Doc entry #26874
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
() Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text: