szehon-ho commented on code in PR #6779:
URL: https://github.com/apache/iceberg/pull/6779#discussion_r1109194175
##########
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/SparkSchemaUtil.java:
##########
@@ -354,4 +354,15 @@ public static Map<Integer, String>
indexQuotedNameById(Schema schema) {
Function<String, String> quotingFunc = name -> String.format("`%s`",
name.replace("`", "``"));
return TypeUtil.indexQuotedNameById(schema.asStruct(), quotingFunc);
}
+
+ /**
+ * Convert a {@link PartitionSpec} to a {@link DataType Spark type}.
+ *
+ * @param spec iceberg PartitionSpec
+ * @return {@link StructType}
+ * @throws IllegalArgumentException if the type cannot be converted
+ */
+ public static StructType convert(PartitionSpec spec) {
+ return convert(new
Schema(spec.partitionType().asNestedType().asStructType().fields()));
Review Comment:
I think the general project direction has been to avoid adding public method
unless we need it in many places. This is converting from a partition spec
type to spark type, and should be fairly limited, so would suggest inline.
I personally think , if anything, adding public method convert(StructType)
to spark StructType would be more useful, and will cover this case here.
Would like to see what @aokolnychyi @RussellSpitzer think as well.
Also, is it more convoluted than needed? Could it be?
```
convert(new Schema(partType.partitionType().fields())))
```
##########
spark/v3.2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java:
##########
@@ -911,6 +935,14 @@ public void
testPartitionedImportFromEmptyPartitionDoesNotThrow() {
new StructField("ts", DataTypes.DateType, true, Metadata.empty())
};
+ private static final StructField[] dateHourStruct = {
+ new StructField("id", DataTypes.IntegerType, true, Metadata.empty()),
+ new StructField("name", DataTypes.StringType, true, Metadata.empty()),
+ new StructField("dept", DataTypes.StringType, true, Metadata.empty()),
+ new StructField("ts", DataTypes.DateType, true, Metadata.empty()),
+ new StructField("hour", DataTypes.StringType, true, Metadata.empty())
Review Comment:
Question, (maybe I am not understanding original problem entirely), can we
re-use existing test struct, ie, using "dept=01" as partition field? I think
we can remove a lot of test code, if so?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
