Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/15730 )
Change subject: IMPALA-9469: ORC scanner vectorization for collection types ...................................................................... Patch Set 2: Code-Review+2 (5 comments) http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15730/1//COMMIT_MSG@32 PS1, Line 32: > Unfortunately there is no 'scale' option for loading the nested workloads. No need to put more efforts into this if there is no convenient way to produce scaled nested workloads. Thx for the explanation! http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h File be/src/exec/orc-column-readers.h: http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.h@95 PS1, Line 95: ReadValue > In that case we'd need more friend declarations, because both OrcListReader Yeah, agree it's more convenient to leave it here. I don't have strong feelings for either option. http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc File be/src/exec/orc-column-readers.cc: http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@451 PS1, Line 451: int OrcListReader::NumElements() const { > Yeah, it's actually the same function. I see. But we still gain something with codegen nested types so it's still a win. Thx for rewriting this. http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@473 PS1, Line 473: > If the list is an array of structs, then we can have many children, but in Indeed. Thx! http://gerrit.cloudera.org:8080/#/c/15730/1/be/src/exec/orc-column-readers.cc@477 PS1, Line 477: return Status::OK(); > Yeah, it's confusing. slot_desc_ being null is independent of MaterializeTu Hmm, According to my understanding MaterializeTuple() is true when the reader writes directly to a slot and false if the reader delegates the writing to its children. I might have misunderstood then. Still, introducing DirectReader() helps readability, thx. -- To view, visit http://gerrit.cloudera.org:8080/15730 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I477961b427406035a04529c5175dbee8f8a93ad5 Gerrit-Change-Number: 15730 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Norbert Luksa <norbert.lu...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Fri, 17 Apr 2020 13:13:34 +0000 Gerrit-HasComments: Yes