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

Reply via email to