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 3: Code-Review+1 (3 comments) LGTM. We can bump it to +2 after resolving the discussions. http://gerrit.cloudera.org:8080/#/c/17039/3/be/src/exec/orc-column-readers.cc File be/src/exec/orc-column-readers.cc: http://gerrit.cloudera.org:8080/#/c/17039/3/be/src/exec/orc-column-readers.cc@308 PS3, Line 308: i.e. STRUCT or MAP I think we should mention that LIST also has one child type but we won't use this function for it. http://gerrit.cloudera.org:8080/#/c/17039/3/be/src/exec/orc-column-readers.cc@321 PS3, Line 321: node nit: should we use DCHECK(node != nullptr) instead? http://gerrit.cloudera.org:8080/#/c/17039/3/be/src/exec/orc-column-readers.cc@728 PS3, Line 728: SchemaPathConstants::MAP_KEY nit: For code readability, I think we should use 0 here, because 'field' is now a subcolumn index of an ORC MAP type instead of a field in the SchemaPath. It's unrelative to SchemaPathConstants anymore. -- 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: 3 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-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 19 Feb 2021 13:39:35 +0000 Gerrit-HasComments: Yes
