Michael Ho has posted comments on this change. Change subject: IMPALA-3962: Clean up scratch tuple batch on scan failures ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/3991/1/be/src/exec/hdfs-parquet-scanner.cc File be/src/exec/hdfs-parquet-scanner.cc: Line 343: context_->ClearStreams(); > why is this under !advance_row_group_? If we did this here, then it seems Yes. I am not 100% sure it's side-effect free (e.g. are these calls idempotent ?). Let me think more about this. Line 373: if (context_->cancelled()) return Status::CANCELLED; > why is this needed? won't the next GetNextInternal() notice this (inside Co In case of parse error in AssembleRows(), we may not have called CommitRows(). However, it may be worth some more rethinking of how the code should be structured so we can avoid this redundant check here. Line 513: *skip_row_group = true; > doesn't this path have the same bug? Yes, missed that after rebase. -- To view, visit http://gerrit.cloudera.org:8080/3991 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If1e27a1517d09ccaabdae1492b7e1fbf661ae3e5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-HasComments: Yes
