dimas-b commented on code in PR #10329:
URL: https://github.com/apache/iceberg/pull/10329#discussion_r1599208577


##########
core/src/test/java/org/apache/iceberg/TestLocationProvider.java:
##########
@@ -285,4 +286,22 @@ public void testObjectStorageWithinTableLocation() {
     assertThat(parts).element(2).asString().isNotEmpty();
     assertThat(parts).element(3).asString().isEqualTo("test.parquet");
   }
+
+  @TestTemplate
+  public void testEncodedFieldNameInPartitionPath() {
+    // Update the table to use a string field for partitioning with special 
characters in the name
+    table.updateProperties().set(TableProperties.OBJECT_STORE_ENABLED, 
"true").commit();
+    table.updateSchema().addColumn("data#1", Types.StringType.get()).commit();
+    table.updateSpec().addField("data#1").commit();

Review Comment:
   Maybe nitpicking, but in my reading of the AWS doc linked above I do not 
think they are recommending URL encoding. The doc's language is pretty vague. 
In fact, I suspect URL encoding even partition key values here will cause 
incorrect processing in tools that use `java.net.URI` for parsing those 
locations in Iceberg metadata files because the escaped chars will be converted 
to their proper Unicode chars during parsing and will likely not match existing 
S3 keys when passed to 
`software.amazon.awssdk.services.s3.S3Utilities.parseUri(URI uri)`... I'm not 
saying there's definitely a bug there, but I strongly suspect interoperability 
issues.



-- 
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: issues-unsubscr...@iceberg.apache.org

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

Reply via email to