jackye1995 commented on code in PR #6772: URL: https://github.com/apache/iceberg/pull/6772#discussion_r1103706868
########## core/src/test/java/org/apache/iceberg/util/TestLocationUtil.java: ########## @@ -61,4 +61,124 @@ public void testStripTrailingSlashWithInvalidPath() { () -> LocationUtil.stripTrailingSlash(invalidPath)); } } + + @Test + public void testPosixNormalizePathsWithSchemeAndAuthorityS3Style() { + Assert.assertEquals( + "Must work with a root authority", + "s3://bucket", + LocationUtil.posixNormalize("s3://bucket")); + Assert.assertEquals( + "Must remove / from the end of authority", + "s3://bucket", + LocationUtil.posixNormalize("s3://bucket/")); + Assert.assertEquals( + "Must remove / from the end of directory", + "s3://bucket/dir", + LocationUtil.posixNormalize("s3://bucket/dir/")); + Assert.assertEquals( + "Must change // to / to enforce posix path", + "s3://bucket/foo/bar", + LocationUtil.posixNormalize("s3://bucket/foo//bar")); + Assert.assertEquals( + "Must resolve .. to previous directory to enforce posix path", + "s3://bucket/bar", + LocationUtil.posixNormalize("s3://bucket/foo/../bar")); + Assert.assertEquals( + "Must resolve . to current directory to enforce posix path", + "s3://bucket/foo/bar", + LocationUtil.posixNormalize("s3://bucket/foo/.//bar")); + } + + @Test + public void testPosixNormalizePathsWithSchemeAndAuthorityHdfsStyle() { + Assert.assertEquals( + "Must work with a root authority", + "hdfs://1.2.3.4", + LocationUtil.posixNormalize("hdfs://1.2.3.4")); + Assert.assertEquals( + "Must remove / from the end of authority", + "hdfs://1.2.3.4", + LocationUtil.posixNormalize("hdfs://1.2.3.4/")); + Assert.assertEquals( + "Must remove / from the end of directory", + "hdfs://1.2.3.4/dir", + LocationUtil.posixNormalize("hdfs://1.2.3.4/dir/")); + Assert.assertEquals( + "Must change // to / to enforce posix path", + "hdfs://1.2.3.4/foo/bar", + LocationUtil.posixNormalize("hdfs://1.2.3.4/foo//bar")); + Assert.assertEquals( + "Must resolve .. to previous directory to enforce posix path", + "hdfs://1.2.3.4/bar", + LocationUtil.posixNormalize("hdfs://1.2.3.4/foo/../bar")); + Assert.assertEquals( + "Must resolve . to current directory to enforce posix path", + "hdfs://1.2.3.4/foo/bar", + LocationUtil.posixNormalize("hdfs://1.2.3.4/foo/.//bar")); + } + + @Test + public void testPosixNormalizePathsWithSchemeAndWithoutAuthority() { + Assert.assertEquals( + "Must work with the root directory representation", + "file:///", Review Comment: This is a place where Hadoop Path behavior differs, it resolves to `file:/`, but that seems like an invalid resolution. Also I am treating `/` as a special case, both here and in a path without a scheme. For paths with a scheme and authority, we can use `scheme://authority` as the root path without ambiguity. But `file://` (just scheme) and ` ` (empty path) do not mean root location, only `file:///` and `/` mean that. -- 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