Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4008: don't bake in hash table and hash join pointers
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4326/4/be/src/exec/hash-table.h
File be/src/exec/hash-table.h:

Line 308:     uint8_t* ALWAYS_INLINE cur_expr_values() const { return 
cur_expr_values_; }
> let's add a short comment here too:
Done


PS4, Line 310: .
> for the current row.
Done


PS4, Line 400: into
> This one should have stayed "in" (or "of"), right? i.e. it doesn't write *e
Done


Line 427:   uint32_t HashVariableLenRow(uint8_t* expr_values, uint8_t* 
expr_values_null) const;
> how about making these const as well.
Done


Line 441:       TupleRow* build_row, uint8_t* expr_values, uint8_t* 
expr_values_null) const;
> these too could be const to clarify they are "in" params.
Done. Also made the various TupleRow arguments const too for consistency. This 
all led to a few additional consts.


-- 
To view, visit http://gerrit.cloudera.org:8080/4326
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie353666dbb5c958f0094d169306fe930ec3014c5
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to