Zoltan Borok-Nagy 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 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/17039/3/be/src/exec/hdfs-orc-scanner.h File be/src/exec/hdfs-orc-scanner.h: http://gerrit.cloudera.org:8080/#/c/17039/3/be/src/exec/hdfs-orc-scanner.h@212 PS3, Line 212: from slot/ > If I saw correctly, we always know exactly whether it is a slot or a tuple Done 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: tiple children, i. > I think we should mention that LIST also has one child type but we won't us Re-worded a bit to emphasize it's for complex types only that might have multiple children. 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? We can have a nullptr here in case of MAPs. It happens when we only query one child of the map. E.g. when we query the "value" of the map, the ORC lib still creates two children of the MAP type, one child being nullptr. I think it's because this way 0 always mean KEY and 1 always mean VALUE. http://gerrit.cloudera.org:8080/#/c/17039/3/be/src/exec/orc-column-readers.cc@728 PS3, Line 728: 0) { > nit: For code readability, I think we should use 0 here, because 'field' is Done -- 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: 4 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 16:18:34 +0000 Gerrit-HasComments: Yes
