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

Reply via email to