impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <>
Subject [Impala-ASF-CR] MPALA-5776: Write partial tuple to the correct mempool
Date Fri, 11 Aug 2017 18:57:02 GMT
Alex Behm has posted comments on this change.

Change subject: MPALA-5776: Write partial tuple to the correct mempool

Patch Set 2:

File be/src/exec/

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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I60ba5c113aefd17f697c1888fd46a237ef396540
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message