Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17039 )

Change subject: IMPALA-10485: part(1): make ORC column reader creation 
independent of schema resolution
......................................................................


Patch Set 1:

(5 comments)

Good to see a negative size patch! Thanks for the simplification! I'm going to 
have a detailed look. Left some minor comments first.

http://gerrit.cloudera.org:8080/#/c/17039/1/be/src/exec/orc-column-readers.cc
File be/src/exec/orc-column-readers.cc:

http://gerrit.cloudera.org:8080/#/c/17039/1/be/src/exec/orc-column-readers.cc@291
PS1, Line 291: inline bool PathContains(const SchemaPath& path, const 
SchemaPath& sub_path) {
             :   return path.size() >= sub_path.size() &&
             :       std::equal(sub_path.begin(), sub_path.end(), path.begin());
             : }
This is no longer used.


http://gerrit.cloudera.org:8080/#/c/17039/1/be/src/exec/orc-column-readers.cc@296
PS1, Line 296: inline const SchemaPath& GetTargetColPath(const SlotDescriptor* 
slot_desc) {
This is no longer used too.


http://gerrit.cloudera.org:8080/#/c/17039/1/be/src/exec/orc-column-readers.cc@311
PS1, Line 311: bool IsDescendant(const orc::Type& node, uint64_t 
candidate_col_id) {
nit: inline?


http://gerrit.cloudera.org:8080/#/c/17039/1/be/src/exec/orc-column-readers.cc@316
PS1, Line 316:
Could you move the original comments of FindChild here?


http://gerrit.cloudera.org:8080/#/c/17039/1/be/src/exec/orc-column-readers.cc@606
PS1, Line 606:   // We have a position slot descriptor if it refers to this 
LIST ORC type, but it isn't
             :   // a collection slot.
It may be useful to mention "See how position slots are resolved in 
HdfsOrcScanner::ResolveColumns()."



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f7d521f9397c5188fadc7996cee0bd1650d363e
Gerrit-Change-Number: 17039
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Wed, 10 Feb 2021 09:57:06 +0000
Gerrit-HasComments: Yes

Reply via email to