rdblue commented on a change in pull request #1842:
URL: https://github.com/apache/iceberg/pull/1842#discussion_r536475979
##########
File path:
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTypeConverter.java
##########
@@ -30,7 +37,52 @@ private HiveTypeConverter() {
}
- public static String convert(Type type) {
+ /**
+ * Converts the Iceberg schema to a Hive schema.
+ * @param schema The original Iceberg schema to convert
+ * @return The Hive column list generated from the Iceberg schema
+ */
+ public static List<FieldSchema> hiveSchema(Schema schema) {
Review comment:
Most of the other modules use a different naming convention:
* `AvroSchemaUtil.convert` for Schema to Iceberg type and Iceberg type to
Schema
* `ParquetSchemaUtil.convert`
* `SparkSchemaUtil.convert`
* `FlinkSchemaUtil.convert`
I think it would be nice to use the same convention here: use
`HiveSchemaUtil` instead of `HiveTypeConverter` and then use overloads of
`convert`. This would also make the API a bit less confusing because
`HiveTypeConverter` and `HiveSchemaConverter` are very similar names and share
a lot of the same tasks with different method args. Could these be combined
into one?
Some schema classes have a different type for the top-level schema (Iceberg
and Parquet) and we usually convert between the two. Here, I think you could
convert between Iceberg `Schema` and `List<FieldSchema>`, and between Iceberg
`Type` and `TypeInfo`.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
