gszadovszky commented on code in PR #3287:
URL: https://github.com/apache/parquet-java/pull/3287#discussion_r2324255425


##########
parquet-cli/pom.xml:
##########
@@ -85,6 +85,26 @@
       <artifactId>parquet-avro</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <groupId>org.apache.parquet</groupId>
+      <artifactId>parquet-protobuf</artifactId>
+      <version>${project.version}</version>
+      <classifier>tests</classifier>
+      <scope>test</scope>
+    </dependency>
+    <!-- CatCommandTest (for TestProtobuf) -->

Review Comment:
   I think I was wrong with my previous comment. We do not actually use 
protobuf in prod, right? We check only for the related key in the footer and 
use the example binding to get the values. So `test` scope seems legit.
   (If you want to keep this comment, maybe let it be for all three 
dependencies with some separation?)



##########
parquet-cli/src/test/java/org/apache/parquet/cli/commands/CatCommandTest.java:
##########
@@ -63,4 +67,42 @@ public void testCatCommandWithInvalidColumn() throws 
IOException {
     command.setConf(new Configuration());
     command.run();
   }
+
+  @Test
+  public void testCatCommandProtoParquetAutoDetected() throws Exception {
+    File protoFile = new File(getTempFolder(), "proto_someevent.parquet");
+    writeProtoParquet(protoFile);
+
+    CatCommand cmd = new CatCommand(createLogger(), 0);
+    cmd.sourceFiles = Arrays.asList(protoFile.getAbsolutePath());
+    cmd.setConf(new Configuration());
+
+    int result = cmd.run();
+    Assert.assertEquals(0, result);
+  }
+
+  @Test
+  public void testCatCommandProtoParquetSucceedsWithAutoDetection() throws 
Exception {

Review Comment:
   Aren't the two tests the same? Maybe we should test the new config as well. 
(AFAIK the avro reader fails with INT96 values by default.)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to