Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/15370 )
Change subject: IMPALA-6636: Use async IO in ORC scanner ...................................................................... Patch Set 16: (7 comments) Thank you, Kurt! Patch set 16 address some of your comments. http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/hdfs-columnar-scanner.h File be/src/exec/hdfs-columnar-scanner.h: http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/hdfs-columnar-scanner.h@87 PS15, Line 87: /// current columns being scanned. Sets the reservation on each corresponding reader > Add typedef/using for template types here and below. Done http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/hdfs-columnar-scanner.cc File be/src/exec/hdfs-columnar-scanner.cc: http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/hdfs-columnar-scanner.cc@151 PS15, Line 151: int64_t right_len = col_range_lengths[right.first]; > Parenthesize conditions for readability Done http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/hdfs-orc-scanner.h File be/src/exec/hdfs-orc-scanner.h: http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/hdfs-orc-scanner.h@95 PS15, Line 95: uint64_t offset_; > Use signed types per style guide? We use uint64_t in this struct to match with ScanRangeInputStream and orc::InputStream from ORC lib. I think we should keep this unsigned. Otherwise, we need to keep doing signed to unsigned type casting in HdfsOrcScanner::ColumnRange::read. http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/hdfs-orc-scanner.cc File be/src/exec/hdfs-orc-scanner.cc: http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/hdfs-orc-scanner.cc@111 PS15, Line 111: if (columnRange == nullptr) { > nullptr Done http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/hdfs-orc-scanner.cc@309 PS15, Line 309: : HdfsColumnarScanner(scan_node, state), > Can lower_bound be used here? I change it to use std::find_if. I also inline it at hdfs-orc-scanner.h. http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/parquet/hdfs-parquet-scanner.cc File be/src/exec/parquet/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/15370/15/be/src/exec/parquet/hdfs-parquet-scanner.cc@2881 PS15, Line 2881: ColumnReservations reservation_per_column; > Add typedef/using Done http://gerrit.cloudera.org:8080/#/c/15370/15/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/15370/15/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@385 PS15, Line 385: private boolean hasOrc(Set<HdfsFileFormat> fileFormats) { > declare const This is the frontend java code. So there is no const specifier in method declaration. -- To view, visit http://gerrit.cloudera.org:8080/15370 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I348ad9e55f0cae7dff0d74d941b026dcbf5e4074 Gerrit-Change-Number: 15370 Gerrit-PatchSet: 16 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Mon, 06 Dec 2021 20:39:45 +0000 Gerrit-HasComments: Yes
