StefanXiepj commented on a change in pull request #849:
URL: https://github.com/apache/parquet-mr/pull/849#discussion_r544782736



##########
File path: 
parquet-tools/src/main/java/org/apache/parquet/tools/command/DumpCommand.java
##########
@@ -165,48 +165,47 @@ public static void dump(PrettyPrintWriter out, 
ParquetMetadata meta, MessageType
 
         ParquetFileReader freader = null;
         if (showmd) {
+          long group = 0;
+          for (BlockMetaData block : blocks) {
             try {
-                long group = 0;
-                for (BlockMetaData block : blocks) {
-                    if (group != 0) out.println();
-                    out.format("row group %d%n", group++);
-                    out.rule('-');
-
-
-                    List<ColumnChunkMetaData> ccmds = block.getColumns();
-                    if (showColumns != null) {
-                        ccmds = new ArrayList<ColumnChunkMetaData>();
-                        for (ColumnChunkMetaData ccmd : block.getColumns()) {
-                            String path = 
Joiner.on('.').skipNulls().join(ccmd.getPath().toArray());
-                            if (showColumns.contains(path)) {
-                                ccmds.add(ccmd);
-                            }
-                        }
-                    }
-
-                    MetadataUtils.showDetails(out, ccmds);
+              if (group != 0)
+                out.println();
+              out.format("row group %d%n", group++);
+              out.rule('-');
+
+              List<ColumnChunkMetaData> ccmds = block.getColumns();
+              if (showColumns != null) {
+                ccmds = new ArrayList<ColumnChunkMetaData>();
+                for (ColumnChunkMetaData ccmd : block.getColumns()) {
+                  String path = 
Joiner.on('.').skipNulls().join(ccmd.getPath().toArray());
+                  if (showColumns.contains(path)) {
+                    ccmds.add(ccmd);
+                  }
+                }
+              }
 
-                    List<BlockMetaData> rblocks = 
Collections.singletonList(block);
-                    freader = new ParquetFileReader(
-                        conf, meta.getFileMetaData(), inpath, rblocks, 
columns);
-                    PageReadStore store = freader.readNextRowGroup();
-                    while (store != null) {
-                        out.incrementTabLevel();
-                        for (ColumnDescriptor column : columns) {
-                            out.println();
-                            dump(out, store, column);
-                        }
-                        out.decrementTabLevel();
+              MetadataUtils.showDetails(out, ccmds);
 
-                        store = freader.readNextRowGroup();
-                    }
-                    out.flushColumns();
+              List<BlockMetaData> rblocks = Collections.singletonList(block);
+              freader = new ParquetFileReader(conf, meta.getFileMetaData(), 
inpath, rblocks, columns);
+              PageReadStore store = freader.readNextRowGroup();
+              while (store != null) {
+                out.incrementTabLevel();
+                for (ColumnDescriptor column : columns) {
+                  out.println();
+                  dump(out, store, column);
                 }
+                out.decrementTabLevel();
+
+                store = freader.readNextRowGroup();
+              }
+              out.flushColumns();
             } finally {
-                if (freader != null) {
-                    freader.close();
-                }
+              if (freader != null) {
+                freader.close();

Review comment:
       yeah yeah,  thx for your review, i am sorry, please forgive me. Do you 
mean we can fix it like this:
   ```
   ...
               } finally {
                 if (freader != null) {
                   freader.close();
                   // reset current reader is null, avoid closing a reader twice
                   freader = null;
                 }
               }
   ```
   or
   ```
   try(ParquetFileReader freader = new ParquetFileReader(conf, 
meta.getFileMetaData(), inpath, rblocks, columns)){
   // do something where the freader is really needed.
   }
   ```
   is right ?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to