rdblue commented on a change in pull request #1632:
URL: https://github.com/apache/iceberg/pull/1632#discussion_r507895309
##########
File path:
mr/src/main/java/org/apache/iceberg/mr/hive/serde/objectinspector/IcebergObjectInspector.java
##########
@@ -36,23 +37,29 @@
// get the correct inspectors depending on whether we're working with Hive2
or Hive3 dependencies
// we need to do this because there is a breaking API change in
Date/TimestampObjectInspector between Hive2 and Hive3
- private static final ObjectInspector DATE_INSPECTOR =
DynMethods.builder("get")
-
.impl("org.apache.iceberg.mr.hive.serde.objectinspector.IcebergDateObjectInspectorHive3")
-
.impl("org.apache.iceberg.mr.hive.serde.objectinspector.IcebergDateObjectInspector")
- .buildStatic()
- .invoke();
-
- private static final ObjectInspector TIMESTAMP_INSPECTOR =
DynMethods.builder("get")
-
.impl("org.apache.iceberg.mr.hive.serde.objectinspector.IcebergTimestampObjectInspectorHive3",
boolean.class)
-
.impl("org.apache.iceberg.mr.hive.serde.objectinspector.IcebergTimestampObjectInspector",
boolean.class)
- .buildStatic()
- .invoke(false);
-
- private static final ObjectInspector TIMESTAMP_INSPECTOR_WITH_TZ =
DynMethods.builder("get")
-
.impl("org.apache.iceberg.mr.hive.serde.objectinspector.IcebergTimestampObjectInspectorHive3",
boolean.class)
-
.impl("org.apache.iceberg.mr.hive.serde.objectinspector.IcebergTimestampObjectInspector",
boolean.class)
- .buildStatic()
- .invoke(true);
+ private static final ObjectInspector DATE_INSPECTOR =
getObjectInspectorBasedOnHiveVersion("get",
+
"org.apache.iceberg.mr.hive.serde.objectinspector.IcebergDateObjectInspector",
+
"org.apache.iceberg.mr.hive.serde.objectinspector.IcebergDateObjectInspectorHive3",
+ new Class[] {}, new Object[] {});
+
+ private static final ObjectInspector TIMESTAMP_INSPECTOR =
getObjectInspectorBasedOnHiveVersion("get",
+
"org.apache.iceberg.mr.hive.serde.objectinspector.IcebergTimestampObjectInspector",
+
"org.apache.iceberg.mr.hive.serde.objectinspector.IcebergTimestampObjectInspectorHive3",
+ new Class[] { boolean.class }, new Object[] { false });
+
+ private static final ObjectInspector TIMESTAMP_INSPECTOR_WITH_TZ =
getObjectInspectorBasedOnHiveVersion("get",
+
"org.apache.iceberg.mr.hive.serde.objectinspector.IcebergTimestampObjectInspector",
+
"org.apache.iceberg.mr.hive.serde.objectinspector.IcebergTimestampObjectInspectorHive3",
+ new Class[] { boolean.class }, new Object[] { true });
+
+ private static ObjectInspector getObjectInspectorBasedOnHiveVersion(
+ String staticMethod, String hive2Class, String hive3Class, Class[]
argTypes, Object[] args) {
+ String classToUse = MetastoreUtil.hive3PresentOnClasspath() ? hive3Class :
hive2Class;
+ return DynMethods.builder(staticMethod)
+ .impl(classToUse, argTypes)
+ .buildStatic()
+ .invoke(args);
Review comment:
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:
```java
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
