Quanlong Huang 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)

I've added some slides but still have more things to cover: 
https://docs.google.com/presentation/d/1uj8m7y69o47MhpqCc0SJ03GDTtPDrg4m04eAFVmq34A

Will add more slides later.

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.
Done. Some members are relative (reader_, row_reader_, reader_options_, 
row_reader_options_) so I skip the blank lines between them.


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
Done


http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@320
PS18, Line 320: RETURN_IF_ERROR
> nit: braces
Done


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.
Done


http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@411
PS18, Line 411: CreateChildForSlot
> nit: braces
Done


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
Done


http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@467
PS18, Line 467: RETURN_IF_ERROR
> nit: braces
Done


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 i
Done


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
Done


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
Done


http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@539
PS18, Line 539: CreateChildForSlot
> nit: braces
Done


http://gerrit.cloudera.org:8080/#/c/12168/18/be/src/exec/orc-column-readers.cc@540
PS18, Line 540: CreateChildForSlot
> nit: braces
Done


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
Done


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
Done



--
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: Mon, 04 Mar 2019 13:40:49 +0000
Gerrit-HasComments: Yes

Reply via email to