Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12168 )
Change subject: IMPALA-6503: Support reading complex types from ORC ...................................................................... Patch Set 18: (14 comments) Found some style-violations. Besides those the reader and scanner code looks good to me. On Monday I'll try to take a deeper look at the OrcSchemaResolver. Thanks for the google doc and slides that explain the details! http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/hdfs-orc-scanner.h File be/src/exec/hdfs-orc-scanner.h: http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/hdfs-orc-scanner.h@178 PS18, Line 178: std::unordered_set<const SlotDescriptor*> missing_field_slots_; Please add comment to this member. Nit: please insert blank lines after member declarations. http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc File be/src/exec/orc-column-readers.cc: http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@287 PS18, Line 287: continue nit: please put it into braces http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@320 PS18, Line 320: RETURN_IF_ERROR nit: braces http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@398 PS18, Line 398: CreateChildForSlot(node, child_slot); nit: put into braces. http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@411 PS18, Line 411: CreateChildForSlot nit: braces http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@451 PS18, Line 451: RETURN_IF_ERROR(child->ReadValue(array_start_ + array_idx_, tuple, pool)); nit: put it into braces http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@467 PS18, Line 467: RETURN_IF_ERROR nit: braces http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@502 PS18, Line 502: (field == SchemaPathConstants::MAP_KEY) key_readers_.push_back(child); : else value_readers_.push_back(chil nit: the whole 'if' statement doesn't fit into one line, so please put it into braces. http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@512 PS18, Line 512: CreateChildForSlot(node, child_slot); nit: braces http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@527 PS18, Line 527: y_selected) key_readers_.push_back(child); : else value_readers_ nit: braces http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@539 PS18, Line 539: CreateChildForSlot nit: braces http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@540 PS18, Line 540: CreateChildForSlot nit: braces http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@579 PS18, Line 579: RETURN_IF_ERROR(child->ReadValue(array_offset_, tuple, pool)); nit: braces http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@590 PS18, Line 590: RETURN_IF_ERROR(child->ReadValue(offset + tuple_idx, tuple, pool)); nit: braces -- To view, visit http://gerrit.cloudera.org:8080/12168 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I244dc9d2b3e425393f90e45632cb8cdbea6cf790 Gerrit-Change-Number: 12168 Gerrit-PatchSet: 18 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 01 Mar 2019 17:26:05 +0000 Gerrit-HasComments: Yes
