unikdahal commented on code in PR #5651:
URL: https://github.com/apache/datafusion-comet/pull/5651#discussion_r3919497917
##########
spark/src/test/scala/org/apache/comet/CometIcebergWriteActionSuite.scala:
##########
@@ -1172,6 +1172,59 @@ class CometIcebergWriteActionSuite
}
}
+ // iceberg-java's `PartitionSpec.partitionToPath` runs every partition name
and value through
+ // `URLEncoder.encode`, and iceberg-rust's `partition_to_path` uses the same
+ // application/x-www-form-urlencoded table (apache/iceberg-rust#2875).
Readers resolve files
+ // through manifest metadata rather than paths, but the committed location
still has to be one
+ // every FileIO can open: an unescaped `#` in an S3 key is a fragment
delimiter to `S3FileIO`.
+ test("native acceleration: partition paths are URL-escaped like
iceberg-java") {
+ assumeNativeAcceleration()
+ withIcebergCatalog { warehouseDir =>
+ createTable(warehouseDir, "escaped_native", partitionSpec = "PARTITIONED
BY (region)")
+ createTable(warehouseDir, "escaped_jvm", partitionSpec = "PARTITIONED BY
(region)")
+ val regions = Seq("a/b", "c#d", "e?f", "g h", "i=j", "k%l", "m+n",
"*-._", "日本", "")
+ val values = regions.zipWithIndex
+ .map { case (region, i) => s"($i, '$region', $i.5)" }
+ .mkString(", ")
+
+ spark.sql(s"INSERT INTO $catalog.$ns.escaped_native VALUES $values")
Review Comment:
Is this actually exercising the native writer?
`COMET_ICEBERG_NATIVE_WRITE_ENABLED` is off by default, and the existing
native-write tests use `assertNativeWriteEngages` / `withNativeEnabled`.
Without that, this could become JVM-vs-JVM and still pass with the old
iceberg-rust revision. Could we use `assertNativeWriteEngages` here?
--
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]
