-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Hive: Add ObjectInspector implementations for UUID, Fixed and Time type #2077
Conversation
@pvary @marton-bod If you have some spare time, could you please have a look at this PR? Thanks |
.../main/java/org/apache/iceberg/mr/hive/serde/objectinspector/IcebergFixedObjectInspector.java
Show resolved
Hide resolved
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
Show resolved
Hide resolved
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
Show resolved
Hide resolved
...t/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergFixedObjectInspector.java
Show resolved
Hide resolved
...st/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergTimeObjectInspector.java
Show resolved
Hide resolved
...st/java/org/apache/iceberg/mr/hive/serde/objectinspector/TestIcebergUUIDObjectInspector.java
Show resolved
Hide resolved
@lcspinter: Thanks for the PR and the fixes. LGTM +1 non-binding |
Looks like this needs to be updated after merging the fix to use source object inspectors. It should probably have the same fix applied, too? |
7c1ca44
to
420ec06
Compare
420ec06
to
3ba4dbb
Compare
ByteBuffer copy = ByteBuffer.wrap(((ByteBuffer) o).array(), ((ByteBuffer) o).arrayOffset(), | ||
((ByteBuffer) o).limit()); | ||
ByteBuffer copy = | ||
ByteBuffer.wrap(((ByteBuffer) o).array(), ((ByteBuffer) o).arrayOffset(), ((ByteBuffer) o).limit()); |
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.
Does this just change formatting? If so, please revert it.
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.
No need, since this is the only issue, I'll merge with it.
Thanks @lcspinter! |
This PR covers the schema conversion for UUID, Fixed and Time types between Iceberg and Hive.
The following mapping is used: