Skip to content

Conversation

@marton-bod
Copy link
Collaborator

@marton-bod marton-bod commented Dec 9, 2020

  • Break up TimestampObjectInspector classes into two separate classes for (i) normal timestamps and (ii) timestamps with zone (instead of using inner classes)
  • For Hive3, take advantage of its TimestampWithLocalTimeZone type (which Hive2 doesn't have) instead of just converting timestampTz into normal timestamps

@marton-bod
Copy link
Collaborator Author

@pvary can you please review when you have the chance?

@pvary
Copy link
Contributor

pvary commented Dec 10, 2020

Do we need to change HiveSchemaConverter?

@marton-bod
Copy link
Collaborator Author

Do we need to change HiveSchemaConverter?

Yes, good point. I'll modify the HiveSchemaConverter as well as the HiveSchemaUtil classes. Adjusting their unit tests are tricky, because the hive-metastore unit tests are run using Hive2 classes. I'll need to think how to go around that.

@rdblue
Copy link
Contributor

rdblue commented Dec 11, 2020

@pvary, please ping me when this is ready to merge.

Copy link
Contributor

@pvary pvary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @marton-bod!
@rdblue, I think this patch is good to go after the tests are finished

@rdblue rdblue merged commit 4c0bf8a into apache:master Dec 12, 2020
@rdblue
Copy link
Contributor

rdblue commented Dec 12, 2020

Merged. Thanks @marton-bod, and thanks for reviewing, @pvary!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants