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

Reply via email to