Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11575 )
Change subject: IMPALA-6964: Track stats about column and page sizes in Parquet reader ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/11575/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11575/2//COMMIT_MSG@21 PS2, Line 21: the amount of uncompressed data read per column for a scan node > Can you add a HDFS scan node profile snippet to the commit message to show Done http://gerrit.cloudera.org:8080/#/c/11575/2/be/src/exec/hdfs-parquet-scanner.h File be/src/exec/hdfs-parquet-scanner.h: http://gerrit.cloudera.org:8080/#/c/11575/2/be/src/exec/hdfs-parquet-scanner.h@358 PS2, Line 358: static const char* LLVM_CLASS_NAME; > Can you hide these and updating them in a helper method? The column reader Done http://gerrit.cloudera.org:8080/#/c/11575/2/be/src/exec/hdfs-parquet-scanner.cc File be/src/exec/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/11575/2/be/src/exec/hdfs-parquet-scanner.cc@80 PS2, Line 80: uet file $0 has an invalid f > Define in L67 in this file, like the previous counters? Why are some of the Done http://gerrit.cloudera.org:8080/#/c/11575/2/be/src/exec/hdfs-parquet-scanner.cc@142 PS2, Line 142: > Can we make these const strings at the top of the file? I can, but all the other counter names are just defined here. I think because the name only needs to be used once (e.g. only in the Open() method). http://gerrit.cloudera.org:8080/#/c/11575/2/be/src/exec/hdfs-parquet-scanner.cc@285 PS2, Line 285: > We should also track this for the collection columns, no? Do CollectionColumnReaders actually read any data from the Parquet file? My understanding is that they just handle Parquet's repetition / definition levels, right? -- To view, visit http://gerrit.cloudera.org:8080/11575 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I322f9b324b6828df28e5caf79529085c43d7c817 Gerrit-Change-Number: 11575 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Comment-Date: Mon, 22 Oct 2018 16:41:56 +0000 Gerrit-HasComments: Yes
