Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/15051 )
Change subject: IMPALA-9226: Improve string allocations of the ORC scanner ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/15051/2/be/src/exec/orc-column-readers.h File be/src/exec/orc-column-readers.h: http://gerrit.cloudera.org:8080/#/c/15051/2/be/src/exec/orc-column-readers.h@207 PS2, Line 207: /*&& !orc_batch->isEncoded*/ Do we need this condition? http://gerrit.cloudera.org:8080/#/c/15051/2/be/src/exec/orc-column-readers.h@219 PS2, Line 219: char* Don't we usually use uint8_t* to refer to raw bytes? Or is it specifically for strings? http://gerrit.cloudera.org:8080/#/c/15051/2/be/src/exec/orc-column-readers.cc File be/src/exec/orc-column-readers.cc: http://gerrit.cloudera.org:8080/#/c/15051/2/be/src/exec/orc-column-readers.cc@180 PS2, Line 180: index < 0 Index is unsigned so it is never less than zero. Why do we cast currentBatch->index[row_idx] to uint64_t? Can't it remain int64_t? -- To view, visit http://gerrit.cloudera.org:8080/15051 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If2d975946fb6f4104d8dc98895285b3a0c6bef7f Gerrit-Change-Number: 15051 Gerrit-PatchSet: 2 Gerrit-Owner: Norbert Luksa <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Norbert Luksa <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Mon, 20 Jan 2020 12:20:10 +0000 Gerrit-HasComments: Yes
