gaborkaszab commented on code in PR #7744:
URL: https://github.com/apache/iceberg/pull/7744#discussion_r1211530056
##########
core/src/main/java/org/apache/iceberg/DataFiles.java:
##########
@@ -248,6 +273,16 @@ public Builder withPartitionPath(String newPartitionPath) {
return this;
}
+ public Builder withPartitionValues(List<String> partitionValues) {
Review Comment:
There is an existing function to provide the partition to the builder.
DataFiles.Builder.withPartition(). I think we shouldn't extend the Builder with
a new function for his purpose.
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMigrateTableProcedure.java:
##########
@@ -184,4 +184,21 @@ public void testInvalidMigrateCases() {
"Cannot handle an empty identifier",
() -> sql("CALL %s.system.migrate('')", catalogName));
}
+
+ @Test
+ public void testMigratePartitionWithSpecialCharacter() throws IOException {
+ Assume.assumeTrue(catalogName.equals("spark_catalog"));
+ String location = temp.newFolder().toString();
+ sql(
+ "CREATE TABLE %s (id bigint NOT NULL, data string) USING parquet "
+ + "PARTITIONED BY (data) LOCATION '%s'",
Review Comment:
You get an exception if you partition by a non-string column. The reason is
that you pass the partition values as a Strin-String map and these string are
not converted into the actual partition field type by default. My
https://github.com/apache/iceberg/pull/7738 has a solution for this.
--
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]
