dimas-b commented on code in PR #10329: URL: https://github.com/apache/iceberg/pull/10329#discussion_r1602092875
########## api/src/main/java/org/apache/iceberg/PartitionSpec.java: ########## @@ -189,7 +189,7 @@ public String partitionToPath(StructLike data) { if (i > 0) { sb.append("/"); } - sb.append(field.name()).append("=").append(escape(valueString)); + sb.append(escape(field.name())).append("=").append(escape(valueString)); Review Comment: > I'm not convinced that we want to move away from a reversible encoding because there's an existing expectation that you could recover by inferring the partitioning from the path structure. Doesn't this PR make the recovery situation worse? For example, in an old path, one could have `s3://bucket/path/id%20=1/...` (no encoding), in the new path it will be `s3://bucket/path/id%2520=1/...`... How does one figure out what to decode and when? -- 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