Tim Armstrong has posted comments on this change. Change subject: IMPALA-4893: Efficiently update the rows read counter for sequence file ......................................................................
Patch Set 1: (1 comment) Can you add a test for the RowsRead counter? It would be nice extra coverage that I think we're currently missing. E.g. I think in scanners.test we do some full table scans of some functional tables, and that is run for all file formats. It looks like runtime_filters.test has some verification of RowsRead. http://gerrit.cloudera.org:8080/#/c/6522/1/be/src/exec/hdfs-sequence-scanner.cc File be/src/exec/hdfs-sequence-scanner.cc: Line 346: COUNTER_ADD(scan_node_->rows_read_counter(), num_rows_read); I think we can avoid the duplicated logic if we break here instead of returning. I.e. if (stream->eof()) break; -- To view, visit http://gerrit.cloudera.org:8080/6522 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie42c97a36e46172884cc497aa645036c2c11f541 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadke <apha...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
