stevenzwu commented on code in PR #13859: URL: https://github.com/apache/iceberg/pull/13859#discussion_r2334065347
########## spark/v4.0/build.gradle: ########## @@ -117,6 +117,7 @@ project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}") { } testImplementation libs.sqlite.jdbc testImplementation libs.awaitility + testImplementation(testFixtures(project(':iceberg-parquet'))) Review Comment: do we intend to remove the existing resources file in 4.0 module in a separate PR? ########## spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/data/vectorized/parquet/TestParquetVectorizedReads.java: ########## @@ -493,6 +496,20 @@ static Stream<Arguments> goldenFilesAndEncodings() { encoding, e.getKey(), e.getValue(), vectorized)))); } + private File resourceUrlToLocalFile(URL url) throws IOException, URISyntaxException { + if ("file".equals(url.getProtocol())) { + return Paths.get(url.toURI()).toFile(); + } Review Comment: nit: Iceberg style is to have an empty line after a control block. ########## spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/data/vectorized/parquet/TestParquetVectorizedReads.java: ########## @@ -493,6 +496,20 @@ static Stream<Arguments> goldenFilesAndEncodings() { encoding, e.getKey(), e.getValue(), vectorized)))); } + private File resourceUrlToLocalFile(URL url) throws IOException, URISyntaxException { + if ("file".equals(url.getProtocol())) { + return Paths.get(url.toURI()).toFile(); + } + String name = Paths.get(url.getPath()).getFileName().toString(); // e.g., string.parquet + String suffix = name.contains(".") ? name.substring(name.lastIndexOf('.')) : ""; + Path tmp = java.nio.file.Files.createTempFile("golden-", suffix); Review Comment: can you explain why do we need to create a tmp file and copy the content? Probably a related question is why do we have 2 different code paths for the condition in line 500 above? ########## spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/data/vectorized/parquet/TestParquetVectorizedReads.java: ########## @@ -493,6 +496,20 @@ static Stream<Arguments> goldenFilesAndEncodings() { encoding, e.getKey(), e.getValue(), vectorized)))); } + private File resourceUrlToLocalFile(URL url) throws IOException, URISyntaxException { + if ("file".equals(url.getProtocol())) { + return Paths.get(url.toURI()).toFile(); + } + String name = Paths.get(url.getPath()).getFileName().toString(); // e.g., string.parquet + String suffix = name.contains(".") ? name.substring(name.lastIndexOf('.')) : ""; + Path tmp = java.nio.file.Files.createTempFile("golden-", suffix); + try (InputStream in = url.openStream()) { + java.nio.file.Files.copy(in, tmp, REPLACE_EXISTING); + } Review Comment: nit: Iceberg style is to have an empty line after a control block. -- 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