Henry Robinson has posted comments on this change. Change subject: Build a ConcatenatedStreams wrapper for ScannerContext::Stream ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/7513/1/be/src/exec/scanner-context.cc File be/src/exec/scanner-context.cc: PS1, Line 335: DCHECK(ValidateCurrentIndex()); Just a passing comment - if this DCHECK fails, all it will tell us via the logs is that ValidateCurrentIndex() was false, because the DCHECK will just print the condition that failed. This is useful for debugging, but not as useful as knowing *how* it failed (off by one error? was the idx badly initialized?). So my suggestion is to have ValidateCurrentIndex() look more like this: void ValidateCurrentIndex() { DCHECK_LE(0, current_idx_); DCHECK_GT(streams_.size(), current_idx_); } The compiler will figure out in release mode that the method call does nothing, and should be able to optimize it out. http://gerrit.cloudera.org:8080/#/c/7513/1/be/src/exec/scanner-context.h File be/src/exec/scanner-context.h: PS1, Line 332: return current_idx_ consider case where current_idx_ < 0 as well? -- To view, visit http://gerrit.cloudera.org:8080/7513 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I026f7a005ba4d6fc412ca0884f4fa0459da9c885 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Pooja Nilangekar <pooja.nilange...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Pooja Nilangekar <pooja.nilange...@cloudera.com> Gerrit-HasComments: Yes
