Taras Bobrovytsky has posted comments on this change. Change subject: MPALA-5776: Write partial tuple to the correct mempool ......................................................................
Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/7639/5/be/src/exec/hdfs-text-scanner.cc File be/src/exec/hdfs-text-scanner.cc: Line 297: boundary_column_.Clear(); > Can we use the existing CopyBoundaryField() function for consistency? Other I tried it and it doesn't quite work (without clearing the boundary field) This is a slightly different case. CopyBoundaryField() copies the contents of the field followed by the boundary column to memory allocated from the row batch. In this case, after FillColumns(), field_locations is pointing at the data in the boundary column. We can trick CopyBoundaryField() by clearing the boundary column right before CopyBoundaryField(), but it's such a weird thing to do. (It looks weird because we just cleared the boundary column, why are we trying to copy out of it?) Another thing we can do, is check that the field is pointing to a different memory location than the boundary column, but I also think that's weird. I suggest we leave it as is (for me, the way it is now is the least confusing way). What do you think? -- 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: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes