Alex Behm has posted comments on this change. Change subject: MPALA-5776: Write partial tuple to the correct mempool ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/7639/2/be/src/exec/hdfs-text-scanner.cc File be/src/exec/hdfs-text-scanner.cc: Line 269: bool eosr = true; Remove. This not needed/used. Line 299: boundary_column_.Clear(); > This is subtle because the contents of boundary_column_ are needed until Wr What's the point of clearing the boundary column here? I don't think the scanner is going to touch it after this point. Line 808: boundary_row_.Reset(); > It might be clearer if we factored out this reset logic for boundary_ and p Another idea: Add a separate function for copying the partial tuple into the output batch. All this logic can go inside there and we can document/DCHECK pre- and post-conditions. Line 896: int num_fields, bool copy_strings) { Can you remove the 'copy_strings' parameter? It's not used in this function and not copying seems too complicated to reason about. -- To view, visit http://gerrit.cloudera.org:8080/7639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
