impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tianyi Wang (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5210: Count rows and collection items in parquet scanner separately
Date Thu, 24 Aug 2017 23:01:24 GMT
Tianyi Wang has posted comments on this change.

Change subject: IMPALA-5210: Count rows and collection items in parquet scanner separately
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7776/2/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

Line 964:   while (!column_readers[0]->RowGroupAtEnd()) {
> Yeah, it changes the semantics to something like "Number of successfully co
Done


http://gerrit.cloudera.org:8080/#/c/7776/3/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

Line 1008:   COUNTER_ADD(scan_node_->rows_read_counter(), num_rows_read);
> You could use COUNTER_SET here and get rid of num_rows_read altogether.
I'm not sure what value should be COUNTER_SET here. Could you elaborate?


PS3, Line 1009: COUNTER_ADD
> How about removing the initialization at the beginning of the method and us
Same as above.


http://gerrit.cloudera.org:8080/#/c/7776/3/be/src/exec/hdfs-parquet-scanner.h
File be/src/exec/hdfs-parquet-scanner.h:

Line 470:   /// Number of collection items read
> Can you describe the scope of this in the comment, e.g. "Total number of co
Updated. In current code it's total number of collection items read in current row batch.
Do you suggest let it count throughout the lifetime of this scanner and update the scannode
counter when this scanner is cleaned up?


http://gerrit.cloudera.org:8080/#/c/7776/3/be/src/exec/scan-node.h
File be/src/exec/scan-node.h:

Line 153:   /// # collection items read from the scanner
> Can you explain here that this is across nested collections, so that A = {B
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7776
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f6efddaea18507482940f5bdab7326b6482b067
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <twang@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <twang@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message