Skip to content

Select correct ObjectInspector based on Hive version on classpath#1632

Merged
rdblue merged 2 commits intoapache:masterfrom
marton-bod:fix_hive2
Oct 19, 2020
Merged

Select correct ObjectInspector based on Hive version on classpath#1632
rdblue merged 2 commits intoapache:masterfrom
marton-bod:fix_hive2

Conversation

@marton-bod
Copy link
Collaborator

To fix #1630, the ObjectInspector selection logic needs to depend on checking the presence of Hive3 classes on the classpath.
cc @rdblue @massdosage @pvary - thanks for checking!

Copy link
Contributor

@massdosage massdosage left a comment

Choose a reason for hiding this comment

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

I just tested this on our cluster and it fixes the issue I reported.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like it would be a bit easier to read if you didn't wrap the DynMethods call and instead just swapped out the class:

private static final TIMESTAMP_INSPECTOR_CLASS = MetastoreUtil.hive3PresentOnClasspath() ?
    "org.apache.iceberg.mr.hive.serde.objectinspector.IcebergTimestampObjectInspector" :
    "org.apache.iceberg.mr.hive.serde.objectinspector.IcebergTimestampObjectInspectorHive3";
private static final ObjectInspector TIMESTAMP_INSPECTOR = DynMethods.builder("get")
    .impl(TIMESTAMP_INSPECTOR_CLASS, boolean.class)
    .buildStatic()
    .invoke(false);

That way you don't need all of the inline array literals.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep...much simpler, thanks:)
changed it

@rdblue rdblue merged commit 0311c23 into apache:master Oct 19, 2020
@rdblue
Copy link
Contributor

rdblue commented Oct 19, 2020

Thanks for the fix!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hive 3 classes present in hive-iceberg-runtime.jar breaks running on Hive 2

3 participants

Comments