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

Change subject: IMPALA-5717: Support for reading ORC data files
......................................................................


Patch Set 10:

(6 comments)

Major changes based on last patch set
- refactor codegen logics
- add startup option --enable_orc_scanner

http://gerrit.cloudera.org:8080/#/c/9134/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9134/5//COMMIT_MSG@15
PS5, Line 15: er is add
> primitive.
Done


http://gerrit.cloudera.org:8080/#/c/9134/5//COMMIT_MSG@19
PS5, Line 19: Currently, we only support reading primitive types. Writing into 
ORC
> Haha ok, we can't do too much about that :). The ORC project should thank y
Done


http://gerrit.cloudera.org:8080/#/c/9134/5/be/src/exec/hdfs-orc-scanner.cc
File be/src/exec/hdfs-orc-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/9134/5/be/src/exec/hdfs-orc-scanner.cc@259
PS5, Line 259:
> I had a look and I don't think anything should be broken if you just add it
Thanks for checking this! Done.


http://gerrit.cloudera.org:8080/#/c/9134/5/be/src/exec/hdfs-orc-scanner.cc@567
PS5, Line 567:   stripe_rows_read_ += num_rows_read;
> We need to make sure that any functions that need to be used by codegen are
Done. Thanks so much for these clear discription!


http://gerrit.cloudera.org:8080/#/c/9134/5/be/src/exec/hdfs-orc-scanner.cc@590
PS5, Line 590: column for batch,
> Can't the caller do the cast though? My thought was that would make the fun
Sure. Done.


http://gerrit.cloudera.org:8080/#/c/9134/7/be/src/exec/hdfs-orc-scanner.cc
File be/src/exec/hdfs-orc-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/9134/7/be/src/exec/hdfs-orc-scanner.cc@77
PS7, Line 77: result. We throw
> Maybe ThrowMemLimitExceeded()?
Done. Your eyes are sharp! :)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7b6ae4ce3b9ee8125b21993702faa87537790a4
Gerrit-Change-Number: 9134
Gerrit-PatchSet: 10
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Fri, 30 Mar 2018 15:42:34 +0000
Gerrit-HasComments: Yes

Reply via email to